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

Use MissionModelId Class instead of String #1533

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

Mythicaeda
Copy link
Contributor

@Mythicaeda Mythicaeda commented Aug 15, 2024

Description

Related Frontend PR.

Updates the Merlin part of the codebase to use a numerical MissionModelId record instead of a raw String to represent model ids. This makes it consistent with the rest of the codebase and the database.

Breaking change as this replaces the ID! in actions.graphql with Int!. This change may impact users of the following actions:

  • getModelEffectiveArguments
  • getActivityEffectiveArgumentsBulk
  • constraintsDslTypescript
  • validateActivityArguments
  • validateModelArguments

Users will be impacted only if they were using a query variable for the missionModelId argument of these actions. If they were, this can be fixed by changing the type of that query variable from ID! to Int!

Two deprecated actions have also been impacted. getActivityEffectiveArguments and resourceTypes. Use getActivityEffectiveArgumentsBulk in place of getActivityEffectiveArguments. Query the resource_type table in place of the resourceTypes action.

Verification

Unit tests were updated to reflect the change in type.

Documentation

Docs PR

Future work

Change done now as we expect users of the Orchestration Library to be referencing our Plan class

Co-authored-by: Ryan Goetz <[email protected]>
@Mythicaeda Mythicaeda added breaking change A change that will require updating downstream code refactor A code change that neither fixes a bug nor adds a feature labels Aug 15, 2024
@Mythicaeda Mythicaeda self-assigned this Aug 15, 2024
@Mythicaeda Mythicaeda requested a review from a team as a code owner August 15, 2024 19:43
@Mythicaeda Mythicaeda removed this from the FY24 Q4 - Ad Hoc Improvements milestone Aug 15, 2024
Mythicaeda added a commit that referenced this pull request Aug 16, 2024
Change done now as we expect users of the Orchestration Library to be referencing our Plan class

Co-authored-by: Ryan Goetz <[email protected]>
@skovati skovati self-requested a review August 20, 2024 20:49
Mythicaeda added a commit that referenced this pull request Aug 20, 2024
Change done now as we expect users of the Orchestration Library to be referencing our Plan class

Co-authored-by: Ryan Goetz <[email protected]>
Copy link
Contributor

@goetzrrGit goetzrrGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if I can approve this PR since I helped work on this, but it all looks good from my point of view.

Mythicaeda added a commit that referenced this pull request Aug 21, 2024
Change done now as we expect users of the Orchestration Library to be referencing our Plan class

Co-authored-by: Ryan Goetz <[email protected]>
@Mythicaeda Mythicaeda merged commit 2e6e099 into develop Aug 21, 2024
21 of 22 checks passed
@Mythicaeda Mythicaeda deleted the feat/mission-model-id-number branch August 21, 2024 22:40
@dandelany
Copy link
Collaborator

👍 looked at this with @Mythicaeda on a call also, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change A change that will require updating downstream code refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify how MissionModelIds are parsed in Scheduler and Merlin Bindings
3 participants