-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
Shared scheduling MVP #1149
Shared scheduling MVP #1149
Conversation
4ef5e26
to
dd86dda
Compare
130141f
to
76f0543
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@duranb I think I'm seeing a recurrence of this issue from the Constraints PR... namely:
- Go to the Goals page and make a New goal.
- While creating the goal (before you click your first Save), change the Reference Model in the top right to a mission model in your database.
- Click Save & then check the Reference Model - it has been cleared/reset and now shows "No Model"
- If you make a change to the goal and set the reference model again, it will work - it fails when you try to set a reference model at the time of constraint creation.
After this I wanted to be sure that the reference model actually was being set correctly after a change, and observed this confusing behavior:
- Starting with a Goal which does not have a reference model (eg. the one from the ^previous test)
- Edit the model & change the reference model dropdown to a model in your DB,
- Make a small change to the goal code so that the Save button appears, then save a new version.
- Check the dropdown in the top right - it still shows the reference model
- Refresh the page - it still shows the reference model
- Now back on the Goals page, find the Goal you just saved and Edit it again
- In the editor, the reference model now shows No Model, and still does after a refresh
After discussing with @AaronPlave I think this is actually the intended behavior here, and the reference model just doesn't persist (in the DB). I think this is a bit confusing, but not worth holding up this PR, we can address with a future improvement if necessary.
Right, the goal of shared constrains/scheduling is to disassociate them from plans and models. The reference model is intended to only bring context into the editor for static code analysis and is intentionally not persisted in the db as it once was before. We can definitely change the label if that's confusing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple updates @duranb :
- @AaronPlave and I fixed the issue noted in this comment by updating the Goals form to match the same pattern from Conditions (commit a359c5c)
- However we found another instance of this problem - errors in the Goal editor from TS files not being properly loaded
To reproduce the second issue:
- Make a new goal, associate a reference mission model
- Paste in one of the examples from the Goals docs https://nasa-ammos.github.io/aerie-docs/scheduling/goals/
- Autocorrect/typeahead in the editor is working correctly and not showing any errors
- Click Save - now the editor incorrectly shows TS errors for the imported aerie libraries like in Aaron's screenshot
- Close the editor, go back to the Goals, page, re-open this Goal in the editor - the problem still persists
- Change the reference model - when you do this, the problem goes away (temporarily) and there are no TS errors.
@duranb let's chat about this tomorrow to assess the difficulty of fixing this vs. impact of releasing as a known issue or holding off. It would be great to include this in 2.6.0 if possible (releasing tomorrow afternoon)
@dandelany I think we left a comment at the same time lol. The static code analysis will only bring in model specific code analysis to the editor after you select a reference model from the dropdown. The editor itself still has some generic code completion. This is why you might be seeing some code completion, but are still getting errors for defining goals until you select a model temporarily. |
ae6756b
to
41fa3bd
Compare
Discussed with @duranb this morning and the following fixes are planned/in-progress before merge:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All issues addressed, backend has been merged and @duranb got the e2e tests passing again. Thanks! Merging.
* split scheduling into metadata and definition * added AssociationForm and DefinitionEditor components * order analyses by descending id * fix editors not loading in preselected models on page load * filter out scheduling items that are private * make constraints/scheduling columns 50/50 --------- Co-authored-by: Aaron Plave <aaron.plave@jpl.nasa.gov>
** DO NOT MERGE WITHOUT BACKEND **
Requires: NASA-AMMOS/aerie#1315
EDIT:
The backend for this PR currently isn't rebased off of latest develop. Rebasing the backend locally should resolve anyBackend should be updated nowplan_specification
errors.Resolves #1133
This PR delivers an MVP that adds support for the new shared scheduling goals and conditions. It currently only supports being able to specify in plans which goals and conditions to use. Model support will come after this.
Adding a new scheduling goal test:
Adding a new scheduling condition test:
Add a scheduling goal to a plan test:
Add a scheduling condition to a plan test: