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.

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

Update UI to use new constraint_run table #770

Closed
camargo opened this issue Jul 11, 2023 · 3 comments · Fixed by #1429
Closed

Update UI to use new constraint_run table #770

camargo opened this issue Jul 11, 2023 · 3 comments · Fixed by #1429
Assignees
Labels
constraints Anything related to the constraints domain next Near term candidates refactor A code change that neither fixes a bug nor adds a feature

Comments

@camargo
Copy link
Member

camargo commented Jul 11, 2023

Now that we have a constraint_run table via NASA-AMMOS/aerie#1013, we need to update the UI to use it! The 'run constraint' button and status indicator should use the new table to show status.

  • The constraint_run now contains the simulation dataset ID, so we can show the constraint run for the simulation we are looking at (if it exists).
  • It also has a status that tells us if the constraint definition itself is out of date with respect to the constraint run we are viewing. We need some new way to display it.
@camargo camargo added refactor A code change that neither fixes a bug nor adds a feature constraints Anything related to the constraints domain labels Jul 11, 2023
@joswig joswig added the next Near term candidates label Oct 26, 2023
@joswig
Copy link
Collaborator

joswig commented Oct 26, 2023

@cohansen is this still needed?

@cohansen
Copy link
Contributor

@cohansen is this still needed?

It depends on how we want the UI to work in respect to the caching. Right now we pull from the cache if there's data available but the user doesn't have any visibility to us doing to, to them it looks like the constraints were run normally.

I think we need to decide if we want to show something about cache data being available in the constraint checking dropdown otherwise we can close this ticket.

@mattdailis
Copy link
Collaborator

mattdailis commented Oct 31, 2023

It depends on how we want the UI to work in respect to the caching

Building off of what Cody said, I think this is somewhat related to #862 and #677. Our current behavior is that the constraints menu bar shows no icon at first, and then switches to a green check when constraints have been run (even if there were violations)

Screenshot 2023-10-31 at 8 55 01 AM Screenshot 2023-10-31 at 8 57 54 AM

A similar behavior occurs in the side panel. Note that the constraint has a little green check mark even though constraints haven't been checked yet.

Screenshot 2023-10-31 at 8 58 29 AM Screenshot 2023-10-31 at 8 58 46 AM

Where this current ticket would come into play is if, on plan load, we want to display the results of checking constraints without the user having to click the "check constraints" button.

EDIT: Another interaction to consider is what should happen when someone else runs constraints, we may want to propagate that information to everyone who's looking at the plan 🤔

@duranb duranb added this to Aerie Jun 21, 2024
@github-project-automation github-project-automation bot moved this to Todo in Aerie Jun 21, 2024
@duranb duranb moved this from Todo to In Progress in Aerie Jun 26, 2024
@duranb duranb moved this from In Progress to Todo in Aerie Jul 10, 2024
@duranb duranb moved this from Todo to In Progress in Aerie Aug 15, 2024
@duranb duranb moved this from In Progress to In Review in Aerie Aug 20, 2024
@github-project-automation github-project-automation bot moved this from In Review to Done in Aerie Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
constraints Anything related to the constraints domain next Near term candidates refactor A code change that neither fixes a bug nor adds a feature
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants