NASA-AMMOS / aerie-ui

The client application for Aerie.
https://nasa-ammos.github.io/aerie-docs/
MIT License
28 stars 5 forks source link

Shared scheduling MVP #1149

Closed duranb closed 5 months ago

duranb commented 5 months ago

DO NOT MERGE WITHOUT BACKEND

Requires: https://github.com/NASA-AMMOS/aerie/pull/1315

EDIT: The backend for this PR currently isn't rebased off of latest develop. Rebasing the backend locally should resolve any plan_specification errors. Backend should be updated now

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:

  1. Create a scheduling goal (note: a model/plan selection is no longer present)
  2. Add some tags
  3. Select the visibility
  4. Fill out the goal definition code
  5. Optional: If you have a model already added, you can select it at the top right of the editor to bring in code completion and analysis (note: this is not the same as selecting a model to assign to the goal)
  6. Click "Save" button

Adding a new scheduling condition test:

  1. Create a scheduling condition (note: a model/plan selection is no longer present)
  2. Add some tags
  3. Select the visibility
  4. Fill out the condition definition code
  5. Optional: If you have a model already added, you can select it at the top right of the editor to bring in code completion and analysis (note: this is not the same as selecting a model to assign to the goal)
  6. Click "Save" button

Add a scheduling goal to a plan test:

  1. Navigate to a plan
  2. Open the "Scheduling Goals" panel
  3. Click on "Manage Scheduling Goals" to open a modal
  4. Check on the goal that you want to associate to the plan
  5. Click "Update" button
  6. Verify that the selection appears in the scheduling goal panel
  7. Select the version that you want
  8. In the Scheduling Goals panel, click the "Schedule" button
  9. Verify that the appropriate scheduling status is displayed.
  10. In the Scheduling Goals panel, click the "Analyze" button
  11. Verify that the appropriate scheduling status is displayed.

Add a scheduling condition to a plan test:

  1. Navigate to a plan
  2. Open the "Scheduling Conditions" panel
  3. Click on "Manage Scheduling Conditions" to open a modal
  4. Check on the condition that you want to associate to the plan
  5. Click "Update" button
  6. Verify that the selection appears in the scheduling condition panel
  7. Select the version that you want
  8. Verify that the selected version persists
duranb commented 5 months ago

@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.

duranb commented 5 months ago

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:

  1. Make a new goal, associate a reference mission model
  2. Paste in one of the examples from the Goals docs https://nasa-ammos.github.io/aerie-docs/scheduling/goals/
  3. Autocorrect/typeahead in the editor is working correctly and not showing any errors
  4. Click Save - now the editor incorrectly shows TS errors for the imported aerie libraries like in Aaron's screenshot
  5. Close the editor, go back to the Goals, page, re-open this Goal in the editor - the problem still persists
  6. 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)

image

@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.

dandelany commented 5 months ago

Discussed with @duranb this morning and the following fixes are planned/in-progress before merge: