Add default Derivation Group associations for Plans at the Model level by pranav-super · Pull Request #1934 · NASA-AMMOS/plandev-ui

pranav-super · 2026-05-20T23:22:42Z

___REQUIRES_AERIE_PR___="1831"

This is the UI update based on the tables introduced in this Plandev PR.

It introduces a derivation group tab along with the other model association tabs on the models page.
This feature is modeled off of the other associations, but since derivation group associations are much simpler than goal/condition/constraint specifications, is a lot lighter-weight and doesn't precisely match functionality.

There are a few minor unresolved TODOs, namely:

deleting an external source gets blocked if there are any outstanding plan-derivation-group links.
We don't believe this is a necessary check for model-derivation-group links (i.e. if a derivation group is linked to a model, and a planner wants to delete a source from said group, they shouldn't be blocked from doing so). We believe this is an unnecessary check because with a model, there are no activities scheduled off of any sources or external events visible to planners at a model level, so there is no anticipated loss when a source is deleted.
a few formatting issues with the derivation group table on the models page. The table is modeled off of the ManagePlanDerivationGroupsModal, and a lot of the formatting and divs were retained as attempts to format otherwise were unsuccessful.

AaronPlave · 2026-05-27T18:08:40Z

      <RadioButton id="constraint"><div class="association-button">Constraints</div></RadioButton>
      <RadioButton id="goal"><div class="association-button">Goals</div></RadioButton>
      <RadioButton id="condition"><div class="association-button">Conditions</div></RadioButton>
+      <RadioButton id="derivation_group"><div class="association-button">Derivation Groups</div></RadioButton>


Needs styling to make it wider to prevent this overflow.

AaronPlave · 2026-05-27T18:09:32Z

+  export let user: User | null;
+  export let selectedDerivationGroups: string[];
+
+  $: console.log('dgmodelspec -> selectedDerivationGroups', selectedDerivationGroups);


AaronPlave · 2026-05-27T18:14:44Z

Styling seems off for the table - it is getting cut off and I can't select any of the rows via checkbox.

AaronPlave · 2026-05-27T18:17:15Z

+  import SectionTitle from '../ui/SectionTitle.svelte';
+
+  export let user: User | null;
+  export let selectedDerivationGroups: string[];


I can't seem to select a derivation group by clicking on a row in the DG table

Ah - i have to click on the expand button.. any reason not to just use left click to trigger the detail view like we do in the other tables in the other tabs?

No reason in particular. This was modeled after how it works in the plan view (see ManagePlanDerivationGroupsModal.svelte), where there is an expansion button, but that can be updated!

Makes sense - I'd vote to align it with the other tabs in there just for consistency

AaronPlave · 2026-05-27T18:19:35Z

+    featurePermissions.derivationGroupModelLink.canCreate(user) &&
+    featurePermissions.derivationGroupModelLink.canDelete(user);
+
+  // $: selectedDerivationGroups = $selectedModelDerivationGroupNames.reduce(


Can this be removed?

AaronPlave · 2026-05-27T18:29:40Z

  return false;
 }

+function isModelOwner(user: User | null, model: AssetWithOwner<ModelWithOwner> | undefined): boolean {


any reason for this new permission when it just wraps isUserOwner with the same args?

It checks for model ownership specifically, and is used a few times! ModelWithOwner has a different set of properties than PlanWithOwner, so giving it a named function seemed appropriate. With that being said...this function is only used twice, whereas the isPlanOwner analogue is used 44 times. If it seems easier to unwrap this and directly use isUserOwner directly instead of this new function, that can be done, but having a named function seemed cleaner, and useful in case model ownership checking should be useful in future features.

But I guess at the end of the day model ownership is just the same as user ownership? Given that it just wraps a function and doesn't make any modifications I feel like it is fine to keep it as just isUserOwner and modify it if/when user ownership =/ model ownership?

I suppose that's true. But then is the isPlanOwner function necessary? I added this function as an analogue to that one.

AaronPlave · 2026-05-27T18:30:06Z

    version: string;
  } | null = null;
+
+  // to get derivation groups here, just need the store, and the table toggle button sends a request to update it.


comment needed?

AaronPlave · 2026-05-27T18:30:26Z

  async deleteExternalSource(
    externalSources: ExternalSourceSlim[] | null,
-    planDerivationGroupLinks: PlanDerivationGroup[],
+    planDerivationGroupLinks: PlanDerivationGroup[], // TODO: add modelDerivationGroup checking? Don't believe it is necessary.


still a TODO?

This was more of a question to the UI team. When deleting a source, we are stopped if that source is part of a derivation group that is associated with a plan to prevent unexpected effects. I don't think it makes sense to do so when a derivation group is only associated with a model and a source is to be deleted, but I wanted to check if you thought that might be worth adding. I describe this in better detail in the original PR comment/description (the first bullet point)!

Got it - yeah seems unnecessary to check all of that since if i understand this all correctly, we're just setting defaults for newly created plans and not existing plans for DGs? That's what happens for constraints/goals/conditions at least.

Yeah, I'd agree. I'll go ahead and remove the TODO!

AaronPlave · 2026-05-27T18:33:21Z

+      }
+      if (model) {
+        derivationGroupModelLinkErrorStore.set(null);
+        if (plan !== null) {


Should pass plan in here instead of relying on the store. Not sure if any effects rely on this store (something must..) but this will always evaluate to true since the store exists. If you want to get the value of the store you need to use the svelte store get(store) to get the current value. But the pattern we should follow for effects is to pass in whatever args are needed such as plan.

On further review, this check should not be there! Removing...

AaronPlave · 2026-05-27T18:35:18Z

  },
+  CREATE_MODEL_DERIVATION_GROUP: (user: User | null, model: ModelWithOwner): boolean => {
+    return (
+      isUserAdmin(user) || (getPermission([Queries.INSERT_PLAN_DERIVATION_GROUP], user) && isModelOwner(user, model))


Should this be queries.INSERT_MODEL_DERIVATION_GROUP?

AaronPlave · 2026-05-27T18:35:32Z

  },
+  DELETE_MODEL_DERIVATION_GROUP: (user: User | null, model: ModelWithOwner): boolean => {
+    return (
+      isUserAdmin(user) || (getPermission([Queries.DELETE_PLAN_DERIVATION_GROUP], user) && isModelOwner(user, model))


Should this be queries.DELETE_MODEL_DERIVATION_GROUP?

AaronPlave · 2026-05-27T18:37:02Z

      {},
    );
  }
+


Remove extra line

AaronPlave · 2026-05-27T18:37:27Z

  `,

+  DELETE_MODEL_DERIVATION_GROUP: `#graphql
+    mutation DeletePlanExternalSource($where: model_derivation_group_bool_exp!) {


Should be DeleteModelDerivationGroup?

AaronPlave · 2026-05-27T18:38:00Z


+  CREATE_MODEL_DERIVATION_GROUP: `#graphql
+    mutation CreateModelDerivationGroup($source: model_derivation_group_insert_input!) {
+      planExternalSourceLink: ${Queries.INSERT_MODEL_DERIVATION_GROUP}(


should planExternalSourceLink be modelDerivationGroupLink?

AaronPlave

Could you add an e2e test for this workflow? Left some other comments but nothing too big to fix.

AaronPlave · 2026-05-27T18:44:45Z

+  ): Promise<void> {
+    try {
+      if ((model && !queryPermissions.DELETE_MODEL_DERIVATION_GROUP(user, model)) || !model) {
+        throwPermissionError('delete a derivation group from the plan');


should be "...from the model"

AaronPlave · 2026-05-27T18:45:11Z

+  ): Promise<void> {
+    try {
+      if ((model && !queryPermissions.CREATE_MODEL_DERIVATION_GROUP(user, model)) || !model) {
+        throwPermissionError('add a derivation group to the plan');


should be "...to the model"

AaronPlave · 2026-05-27T18:45:33Z

+          const { planExternalSourceLink: sourceAssociation } = data;
+          // If the return was null, do nothing - only act on success or non-null
+          if (sourceAssociation !== null) {
+            logMessage(`Linked derivation group "${derivationGroupName}" to plan "${model.name}" (ID=${model.id}).`);


should be "...to model..."?

AaronPlave · 2026-05-27T18:52:47Z

+      const derivationGroupsToDissociate = initialModelDerivationGroupAssociations.filter(
+        derivationGroup => !selectedDerivationGroups.includes(derivationGroup),
+      );
+      derivationGroupsToDissociate.forEach(derivationGroup =>


We should use a for loop here and below when iterating over these promises and we should await the effect promises

AaronPlave · 2026-05-27T18:54:19Z

+      if (model) {
+        derivationGroupModelLinkErrorStore.set(null);
+        if (plan !== null) {
+          const data = await reqHasura<{


Doesn't this return

modelDerivationGroupLink: { returning: { derivation_group_name: string; model_id: number; }[]; };

AaronPlave · 2026-05-27T18:55:02Z

+            },
+            user,
+          );
+          const { planExternalSourceLink: sourceAssociation } = data;


Should be modelDerivationGroupLink

AaronPlave · 2026-05-27T18:55:26Z

+        if (plan !== null) {
+          const data = await reqHasura<ModelDerivationGroup>(
+            gql.CREATE_MODEL_DERIVATION_GROUP,
+            {


Wouldn't this be:

{ modelDerivationGroupLink: { derivation_group_name: string } | null; }

sonarqubecloud · 2026-05-29T22:38:15Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

psubram3 added 8 commits May 20, 2026 15:57

introduce ModelDerivationGroup type

cd98bc8

add ModelDerivationGroup type to model type

905a46d

add effects for ModelDerivationGroup

ecc1ada

add minor stores to reflect derivation group state

748d024

add derivation_group to Association type

808f5a9

create DGModelSpecification.svelte

5fbd55d

incorporate DGModelSpecification.svelte

8f247cf

update permissions

db7fd4b

pranav-super requested a review from a team as a code owner May 20, 2026 23:22

pranav-super requested review from AaronPlave, duranb and joswig May 20, 2026 23:22

pranav-super had a problem deploying to test-workflow May 20, 2026 23:22 — with GitHub Actions Failure

pranav-super mentioned this pull request May 20, 2026

Add default Derivation Group associations for Plans at the Model level NASA-AMMOS/plandev#1831

Open

pranav-super had a problem deploying to test-workflow May 21, 2026 21:05 — with GitHub Actions Failure

AaronPlave had a problem deploying to test-workflow May 27, 2026 17:21 — with GitHub Actions Failure

AaronPlave reviewed May 27, 2026

View reviewed changes

Comment thread src/components/model/DGModelSpecification.svelte

AaronPlave reviewed May 27, 2026

View reviewed changes

Comment thread src/routes/models/[id]/+page.svelte Outdated

{},

);

}

Copy link
Copy Markdown

Contributor

AaronPlave May 27, 2026

Choose a reason for hiding this comment

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

Remove extra line

AaronPlave reviewed May 27, 2026

View reviewed changes

AaronPlave requested changes May 27, 2026

View reviewed changes

AaronPlave reviewed May 27, 2026

View reviewed changes

minor fixes

c4b0b56

pranav-super had a problem deploying to test-workflow May 28, 2026 20:27 — with GitHub Actions Failure

remove TODO

3f629fa

pranav-super had a problem deploying to test-workflow May 29, 2026 22:35 — with GitHub Actions Failure

Conversation

pranav-super commented May 20, 2026 • edited by AaronPlave Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pranav-super May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pranav-super May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronPlave May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronPlave May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronPlave left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented May 29, 2026

Quality Gate failed

pranav-super commented May 20, 2026 •

edited by AaronPlave

Loading

pranav-super May 28, 2026 •

edited

Loading

pranav-super May 28, 2026 •

edited

Loading

AaronPlave May 27, 2026 •

edited

Loading

AaronPlave May 27, 2026 •

edited

Loading