Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Locking doesn't account for optional models #1486

Closed
danielghost opened this issue Mar 24, 2017 · 3 comments · Fixed by #3146
Closed

Locking doesn't account for optional models #1486

danielghost opened this issue Mar 24, 2017 · 3 comments · Fixed by #3146
Labels
bug

Comments

@danielghost
Copy link
Contributor

This essentially means they can become mandatory.
danielghost added a commit that referenced this issue Mar 24, 2017
@danielghost
Fix for excluding _isOptional models from the locking crtieria. #1486.
020eac0
@danielghost danielghost mentioned this issue Mar 24, 2017
Merged
@moloko
Copy link
Contributor

moloko commented Mar 27, 2017

@danielghost could you give a bit more background/detail on this one?

@danielghost
Copy link
Contributor Author

danielghost commented Apr 13, 2017 •
edited
Loading

Previously the locking condition was just skipping over any unavailable models, as these don't need to be included if they were part of the "_lockedBy" requirement for a particular model. We should also skip over any of the models in the list which are optional, otherwise these become mandatory in regards to unlocking the model being checked.

danielghost added a commit that referenced this issue Jul 14, 2017
@danielghost
Updated amend for #1486.
df85989 
Locking doesn't account for optional models.

Also, fixed an issue created in #1488, where only listening to a models immediate children prevented unlocking models outside of this hierachy. Now should also listen to changes for those models listed in `_lockedBy`.
@moloko
Copy link
Contributor

moloko commented Oct 21, 2020

@danielghost is this still relevant or can it be closed now?

@oliverfoster oliverfoster mentioned this issue May 11, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

issue/1486 Ignore optional source models in locking adaptlearning/adapt_framework
2 participants
@moloko @danielghost