Skip to content

Commit

Permalink
Tag deletion specificity fixes (#1521)
Browse files Browse the repository at this point in the history 
* Fix tag directive tag deletion

* Fix constraint tag deletion specificity

* Fix expansion rule tag deletion specificity

* Fix plan tag deletion specificity

* Fix scheduling condition tag deletion specificity

* Fix scheduling goal tag deletion specificity
  • Loading branch information
@AaronPlave
AaronPlave authored Oct 23, 2024
1 parent 5e8d88a commit 2f304ce
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 58 deletions.
4 changes: 2 additions & 2 deletions src/components/activity/ActivityDirectiveForm.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,8 @@
const {
detail: { tag, type },
} = event;
if (type === 'remove') {
await effects.deleteActivityDirectiveTags([tag.id], user);
if (type === 'remove' && $plan) {
await effects.deleteActivityDirectiveTag(tag.id, activityDirective.id, $plan.id, user);
} else if (type === 'create' || type === 'select') {
let tagsToAdd: Tag[] = [tag];
if (type === 'create') {
Expand Down
2 changes: 1 addition & 1 deletion src/components/expansion/ExpansionRuleForm.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@
const unusedTags = initialRuleTags
.filter(tag => !(ruleTags || []).find(t => tag.id === t.id))
.map(tag => tag.id);
await effects.deleteExpansionRuleTags(unusedTags, user);
await effects.deleteExpansionRuleTags(unusedTags, ruleId, user);
ruleUpdatedAt = updated_at;
savedRule = { ...updatedRule, tags: (ruleTags || []).map(tag => ({ tag })) };
Expand Down
4 changes: 2 additions & 2 deletions src/components/plan/PlanForm.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@
const {
detail: { tag, type },
} = event;
if (type === 'remove') {
await effects.deletePlanTags([tag.id], user);
if (type === 'remove' && plan) {
await effects.deletePlanTag(tag.id, plan.id, user);
} else if (plan && (type === 'create' || type === 'select')) {
let tagsToAdd: Tag[] = [tag];
if (type === 'create') {
Expand Down
4 changes: 2 additions & 2 deletions src/tests/mocks/user/mockUser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const mockUser: User = {
delete_activity_by_pk_reanchor_plan_start_bulk: true,
delete_activity_by_pk_reanchor_to_anchor_bulk: true,
delete_activity_directive: true,
delete_activity_directive_tags: true,
delete_activity_directive_tag: true,
delete_activity_presets_by_pk: true,
delete_command_dictionary_by_pk: true,
delete_constraint_by_pk: true,
Expand All @@ -29,7 +29,7 @@ export const mockUser: User = {
delete_expansion_set_by_pk: true,
delete_mission_model_by_pk: true,
delete_plan_by_pk: true,
delete_plan_tags: true,
delete_plan_tag: true,
delete_preset_to_directive_by_pk: true,
delete_scheduling_condition_by_pk: true,
delete_scheduling_goal_by_pk: true,
Expand Down
65 changes: 33 additions & 32 deletions src/utilities/effects.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,7 @@ const effects = {
): Promise<number | null> {
try {
if (!queryPermissions.CREATE_PLAN_SNAPSHOT_TAGS(user)) {
throwPermissionError('create plan tags');
throwPermissionError('create plan snapshot tags');
}

const data = await reqHasura<{ affected_rows: number }>(gql.CREATE_PLAN_SNAPSHOT_TAGS, { tags }, user);
Expand Down Expand Up @@ -1960,29 +1960,31 @@ const effects = {
return false;
},

async deleteActivityDirectiveTags(ids: Tag['id'][], user: User | null): Promise<number | null> {
async deleteActivityDirectiveTag(
tagId: Tag['id'],
directiveId: ActivityDirectiveId,
planId: number,
user: User | null,
): Promise<number | null> {
try {
if (!queryPermissions.DELETE_ACTIVITY_DIRECTIVE_TAGS(user)) {
if (!queryPermissions.DELETE_ACTIVITY_DIRECTIVE_TAG(user)) {
throwPermissionError('delete activity directive tags');
}

const data = await reqHasura<{ affected_rows: number }>(gql.DELETE_ACTIVITY_DIRECTIVE_TAGS, { ids }, user);
const { delete_activity_directive_tags } = data;
if (delete_activity_directive_tags != null) {
const { affected_rows } = delete_activity_directive_tags;

if (affected_rows !== ids.length) {
throw Error('Some activity directive tags were not successfully deleted');
}

const data = await reqHasura<{ tag_id: number }>(
gql.DELETE_ACTIVITY_DIRECTIVE_TAG,
{ directive_id: directiveId, plan_id: planId, tag_id: tagId },
user,
);
if (data.delete_activity_directive_tags_by_pk != null) {
showSuccessToast('Activity Directive Updated Successfully');
return affected_rows;
return data.delete_activity_directive_tags_by_pk.tag_id;
} else {
throw Error('Unable to delete activity directive tags');
throw Error('Unable to delete activity directive tag');
}
} catch (e) {
catchError('Delete Activity Directive Tags Failed', e as Error);
showFailureToast('Delete Activity Directive Tags Failed');
catchError('Delete Activity Directive Tag Failed', e as Error);
showFailureToast('Delete Activity Directive Tag Failed');
return null;
}
},
Expand Down Expand Up @@ -2420,13 +2422,17 @@ const effects = {
return false;
},

async deleteExpansionRuleTags(ids: Tag['id'][], user: User | null): Promise<number | null> {
async deleteExpansionRuleTags(tagIds: Tag['id'][], ruleId: number, user: User | null): Promise<number | null> {
try {
if (!queryPermissions.DELETE_EXPANSION_RULE_TAGS(user)) {
throwPermissionError('delete expansion rule tags');
}

const data = await reqHasura<{ affected_rows: number }>(gql.DELETE_EXPANSION_RULE_TAGS, { ids }, user);
const data = await reqHasura<{ affected_rows: number }>(
gql.DELETE_EXPANSION_RULE_TAGS,
{ rule_id: ruleId, tag_ids: tagIds },
user,
);
const { delete_expansion_rule_tags } = data;
if (delete_expansion_rule_tags != null) {
const { affected_rows } = delete_expansion_rule_tags;
Expand Down Expand Up @@ -2865,27 +2871,22 @@ const effects = {
}
},

async deletePlanTags(ids: Tag['id'][], user: User | null): Promise<number | null> {
async deletePlanTag(tagId: Tag['id'], planId: number, user: User | null): Promise<number | null> {
try {
if (!queryPermissions.DELETE_PLAN_TAGS(user)) {
throwPermissionError('delete plan tags');
if (!queryPermissions.DELETE_PLAN_TAG(user)) {
throwPermissionError('delete plan tag');
}

const data = await reqHasura<{ affected_rows: number }>(gql.DELETE_PLAN_TAGS, { ids }, user);
const { delete_plan_tags } = data;
if (delete_plan_tags != null) {
const { affected_rows } = delete_plan_tags;
if (affected_rows !== ids.length) {
throw Error('Some plan tags were not successfully deleted');
}
const data = await reqHasura<{ tag_id: number }>(gql.DELETE_PLAN_TAG, { plan_id: planId, tag_id: tagId }, user);
if (data.delete_plan_tags_by_pk != null) {
showSuccessToast('Plan Updated Successfully');
return affected_rows;
return data.delete_plan_tags_by_pk.tag_id;
} else {
throw Error('Unable to delete plan tags');
throw Error('Unable to delete plan tag');
}
} catch (e) {
catchError('Delete Plan Tags Failed', e as Error);
showFailureToast('Delete Plan Tags Failed');
catchError('Delete Plan Tag Failed', e as Error);
showFailureToast('Delete Plan Tag Failed');
return null;
}
},
Expand Down
30 changes: 15 additions & 15 deletions src/utilities/gql.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export enum Queries {
CREATE_SNAPSHOT = 'create_snapshot',
DELETE_ACTIVITY_DELETE_SUBTREE_BULK = 'delete_activity_by_pk_delete_subtree_bulk',
DELETE_ACTIVITY_DIRECTIVES = 'delete_activity_directive',
DELETE_ACTIVITY_DIRECTIVE_TAGS = 'delete_activity_directive_tags',
DELETE_ACTIVITY_DIRECTIVE_TAG = 'delete_activity_directive_tags_by_pk', // pluralization is a typo in the db
DELETE_ACTIVITY_PRESET = 'delete_activity_presets_by_pk',
DELETE_ACTIVITY_REANCHOR_PLAN_START_BULK = 'delete_activity_by_pk_reanchor_plan_start_bulk',
DELETE_ACTIVITY_REANCHOR_TO_ANCHOR_BULK = 'delete_activity_by_pk_reanchor_to_anchor_bulk',
Expand Down Expand Up @@ -53,7 +53,7 @@ export enum Queries {
DELETE_PLAN_COLLABORATOR = 'delete_plan_collaborators_by_pk',
DELETE_PLAN_DERIVATION_GROUP = 'delete_plan_derivation_group',
DELETE_PLAN_SNAPSHOT = 'delete_plan_snapshot_by_pk',
DELETE_PLAN_TAGS = 'delete_plan_tags',
DELETE_PLAN_TAG = 'delete_plan_tags_by_pk', // pluralization is a typo in the db
DELETE_PRESET_TO_DIRECTIVE = 'delete_preset_to_directive_by_pk',
DELETE_SCHEDULING_CONDITION_DEFINITION_TAGS = 'delete_scheduling_condition_definition_tags',
DELETE_SCHEDULING_CONDITION_METADATA = 'delete_scheduling_condition_metadata_by_pk',
Expand Down Expand Up @@ -865,10 +865,10 @@ const gql = {
}
`,

DELETE_ACTIVITY_DIRECTIVE_TAGS: `#graphql
mutation DeleteActivityDirectivesTags($ids: [Int!]!) {
${Queries.DELETE_ACTIVITY_DIRECTIVE_TAGS}(where: { tag_id: { _in: $ids } }) {
affected_rows
DELETE_ACTIVITY_DIRECTIVE_TAG: `#graphql
mutation DeleteActivityDirectivesTag($tag_id: Int!, $directive_id: Int!, $plan_id: Int!) {
${Queries.DELETE_ACTIVITY_DIRECTIVE_TAG}(directive_id: $directive_id, plan_id: $plan_id, tag_id: $tag_id) {
tag_id
}
}
`,
Expand Down Expand Up @@ -959,8 +959,8 @@ const gql = {
`,

DELETE_EXPANSION_RULE_TAGS: `#graphql
mutation DeleteExpansionRuleTags($ids: [Int!]!) {
${Queries.DELETE_EXPANSION_RULE_TAGS}(where: { tag_id: { _in: $ids } }) {
mutation DeleteExpansionRuleTags($tag_ids: [Int!]!, $rule_id: Int!) {
${Queries.DELETE_EXPANSION_RULE_TAGS}(where: { tag_id: { _in: $tag_ids }, rule_id: { _eq: $rule_id } }) {
affected_rows
}
}
Expand Down Expand Up @@ -1116,10 +1116,10 @@ const gql = {
}
`,

DELETE_PLAN_TAGS: `#graphql
mutation DeletePlanTags($ids: [Int!]!) {
${Queries.DELETE_PLAN_TAGS}(where: { tag_id: { _in: $ids } }) {
affected_rows
DELETE_PLAN_TAG: `#graphql
mutation DeletePlanTag($tag_id: Int!, $plan_id: Int!) {
${Queries.DELETE_PLAN_TAG}( tag_id: $tag_id, plan_id: $plan_id) {
tag_id
}
}
`,
Expand Down Expand Up @@ -3498,7 +3498,7 @@ const gql = {
}) {
affected_rows
}
deleteConstraintTags: ${Queries.DELETE_CONSTRAINT_TAGS}(where: { tag_id: { _in: $tagIdsToDelete } }) {
deleteConstraintTags: ${Queries.DELETE_CONSTRAINT_TAGS}(where: { tag_id: { _in: $tagIdsToDelete }, constraint_id: { _eq: $id } }) {
affected_rows
}
}
Expand Down Expand Up @@ -3671,7 +3671,7 @@ const gql = {
}) {
affected_rows
}
deleteSchedulingConditionTags: ${Queries.DELETE_SCHEDULING_CONDITION_METADATA_TAGS}(where: { tag_id: { _in: $tagIdsToDelete } }) {
deleteSchedulingConditionTags: ${Queries.DELETE_SCHEDULING_CONDITION_METADATA_TAGS}(where: { tag_id: { _in: $tagIdsToDelete }, condition_id: { _eq: $id } }) {
affected_rows
}
}
Expand Down Expand Up @@ -3780,7 +3780,7 @@ const gql = {
}) {
affected_rows
}
deleteSchedulingGoalTags: ${Queries.DELETE_SCHEDULING_GOAL_METADATA_TAGS}(where: { tag_id: { _in: $tagIdsToDelete } }) {
deleteSchedulingGoalTags: ${Queries.DELETE_SCHEDULING_GOAL_METADATA_TAGS}(where: { tag_id: { _in: $tagIdsToDelete }, goal_id: { _eq: $id } } ) {
affected_rows
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/utilities/permissions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,8 @@ const queryPermissions: Record<GQLKeys, (user: User | null, ...args: any[]) => b
const queries = [Queries.DELETE_ACTIVITY_DELETE_SUBTREE_BULK];
return isUserAdmin(user) || (getPermission(queries, user) && getRolePlanPermission(queries, user, plan, model));
},
DELETE_ACTIVITY_DIRECTIVE_TAGS: (user: User | null): boolean => {
return isUserAdmin(user) || getPermission([Queries.DELETE_ACTIVITY_DIRECTIVE_TAGS], user);
DELETE_ACTIVITY_DIRECTIVE_TAG: (user: User | null): boolean => {
return isUserAdmin(user) || getPermission([Queries.DELETE_ACTIVITY_DIRECTIVE_TAG], user);
},
DELETE_ACTIVITY_PRESET: (user: User | null, preset: AssetWithOwner<ActivityPreset>): boolean => {
return isUserAdmin(user) || (getPermission([Queries.DELETE_ACTIVITY_PRESET], user) && isUserOwner(user, preset));
Expand Down Expand Up @@ -598,8 +598,8 @@ const queryPermissions: Record<GQLKeys, (user: User | null, ...args: any[]) => b
DELETE_PLAN_SNAPSHOT: (user: User | null): boolean => {
return getPermission([Queries.DELETE_PLAN_SNAPSHOT], user) && isUserAdmin(user);
},
DELETE_PLAN_TAGS: (user: User | null): boolean => {
return isUserAdmin(user) || getPermission([Queries.DELETE_PLAN_TAGS], user);
DELETE_PLAN_TAG: (user: User | null): boolean => {
return isUserAdmin(user) || getPermission([Queries.DELETE_PLAN_TAG], user);
},
DELETE_PRESET_TO_DIRECTIVE: (user: User | null, plan: PlanWithOwners): boolean => {
return (
Expand Down

0 comments on commit 2f304ce

Please sign in to comment.