Skip to content

Commit a91af46

Browse files
committed
#4284 update transformer and comments
1 parent 6b8aa27 commit a91af46

5 files changed

Lines changed: 46 additions & 42 deletions

File tree

src/backend/src/prisma-query-args/rules.query-args.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,12 @@ export const getRulePreviewQueryArgs = () =>
4747
}
4848
});
4949

50+
export type ProjectRuleQueryArgs = ReturnType<typeof getProjectRuleQueryArgs>;
51+
5052
export const getProjectRuleQueryArgs = () =>
5153
Prisma.validator<Prisma.Project_RuleDefaultArgs>()({
5254
include: {
53-
rule: getRulePreviewQueryArgs(),
54-
project: { select: { projectId: true } }
55+
rule: getRulePreviewQueryArgs()
5556
}
5657
});
5758

src/backend/src/prisma/seed.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4785,7 +4785,7 @@ const performSeed: () => Promise<void> = async () => {
47854785
// project rules
47864786
await RulesService.createProjectRule(batman, ner, ruleT211.ruleId, projectHuskies1Id);
47874787

4788-
// mark the leaf rule complete from the project to demonstrate global rule completion
4788+
// mark the leaf rule complete
47894789
await RulesService.setRuleCompletion(batman, ner, ruleT211.ruleId, true, projectHuskies1Id);
47904790

47914791
// Technical Rules Section

src/backend/src/services/rules.services.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -385,8 +385,7 @@ export default class RulesService {
385385
throw new HttpException(400, 'This rule is already associated with the project');
386386
}
387387

388-
// Walk up the parent chain to assign all ancestors of a rule to the project as well.
389-
// visited guards against cycles, which editRule does not currently prevent.
388+
// ensure we assign all ancestors of a rule to the project
390389
const ancestorIds: string[] = [];
391390
const visited = new Set<string>([ruleId]);
392391
let currentParentId = rule.parentRuleId;
@@ -397,20 +396,23 @@ export default class RulesService {
397396
where: { ruleId: currentParentId },
398397
select: { parentRuleId: true, dateDeleted: true }
399398
});
400-
// Stop if the ancestor is missing or deleted - deleted rules should not be assigned to projects
401-
if (!parent || parent.dateDeleted) break;
399+
// rule only displays if the full chain to a top-level rule exists, so a missing or deleted
400+
// ancestor means this rule would not display - do not assign it OR its ancestors to the project
401+
if (!parent) throw new NotFoundException('Rule', currentParentId);
402+
if (parent.dateDeleted) throw new DeletedException('Rule', currentParentId);
402403
ancestorIds.push(currentParentId);
403404
currentParentId = parent.parentRuleId;
404405
}
405406

406-
// Only create ancestors that aren't already assigned to the project to avoid duplicate assignment issues
407+
// skip ancestors already assigned to this project
407408
const existingAncestors = await prisma.project_Rule.findMany({
408409
where: { projectId, ruleId: { in: ancestorIds }, dateDeleted: null },
409410
select: { ruleId: true }
410411
});
411412
const existingAncestorIds = new Set(existingAncestors.map((projectRule) => projectRule.ruleId));
412413
const ancestorsToCreate = ancestorIds.filter((id) => !existingAncestorIds.has(id));
413414

415+
// create all project rules
414416
await prisma.$transaction([
415417
...ancestorsToCreate.map((ancestorId) =>
416418
prisma.project_Rule.create({
@@ -430,6 +432,7 @@ export default class RulesService {
430432
})
431433
]);
432434

435+
// return only original project rule being assigned (leaf rule)
433436
const projectRule = await prisma.project_Rule.findUnique({
434437
where: { ruleId_projectId: { ruleId, projectId } },
435438
...getProjectRuleQueryArgs()
@@ -703,8 +706,8 @@ export default class RulesService {
703706
* @param submitter the user updating the completion
704707
* @param organization the organization of the rule
705708
* @param ruleId the id of the rule to update
706-
* @param isComplete whether the rule is complete
707-
* @param projectId the project the rule was completed from (optional - omitted for general view)
709+
* @param isComplete whether the rule is complete or incomplete
710+
* @param projectId the project the rule was completed from (optional - omitted if updated in general view)
708711
* @returns the rule with updated completion
709712
*/
710713
static async setRuleCompletion(
@@ -715,7 +718,7 @@ export default class RulesService {
715718
projectId?: string
716719
): Promise<SharedRule> {
717720
if (!(await userHasPermission(submitter.userId, organization.organizationId, isLeadership))) {
718-
throw new AccessDeniedException('You do not have permissions to update a rule completion');
721+
throw new AccessDeniedException('You do not have permissions to update rule completion');
719722
}
720723

721724
const rule = await prisma.rule.findUnique({

src/backend/src/transformers/rules.transformer.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Prisma } from '@prisma/client';
22
import { Rule, ProjectRule, Ruleset, RulesetType } from 'shared';
3-
import { RulesetQueryArgs, RulePreviewQueryArgs } from '../prisma-query-args/rules.query-args';
3+
import { RulesetQueryArgs, RulePreviewQueryArgs, ProjectRuleQueryArgs } from '../prisma-query-args/rules.query-args.js';
44

55
export const ruleTransformer = (rule: Prisma.RuleGetPayload<RulePreviewQueryArgs>): Rule => {
66
return {
@@ -36,7 +36,7 @@ export const ruleTransformer = (rule: Prisma.RuleGetPayload<RulePreviewQueryArgs
3636
};
3737
};
3838

39-
export const projectRuleTransformer = (projectRule: any): ProjectRule => {
39+
export const projectRuleTransformer = (projectRule: Prisma.Project_RuleGetPayload<ProjectRuleQueryArgs>): ProjectRule => {
4040
return {
4141
projectRuleId: projectRule.projectRuleId,
4242
rule: ruleTransformer(projectRule.rule),

src/backend/tests/unit/rule.test.ts

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -368,15 +368,15 @@ describe('Create Rules Tests', () => {
368368
});
369369

370370
describe('Create Project Rule', () => {
371-
it('Creates project rules for the full ancestor chain when assigning a nested sub-rule', async () => {
372-
const root = await RulesService.createRule(batman, 'T.1', 'Technical Rules', rulesetId, organization);
371+
it('Creates project rules for all ancestors when assigning deep child rule', async () => {
372+
const topLevelRule = await RulesService.createRule(batman, 'T.1', 'Technical Rules', rulesetId, organization);
373373
const child = await RulesService.createRule(
374374
batman,
375375
'T.1.1',
376376
'Vehicle Requirements',
377377
rulesetId,
378378
organization,
379-
root.ruleId
379+
topLevelRule.ruleId
380380
);
381381
const grandchild = await RulesService.createRule(batman, 'T.1.1.1', 'Wheels', rulesetId, organization, child.ruleId);
382382

@@ -386,46 +386,46 @@ describe('Create Rules Tests', () => {
386386

387387
const projectRules = await RulesService.getProjectRules(rulesetId, project.projectId, organization);
388388
const assignedRuleIds = projectRules.map((pr) => pr.rule.ruleId);
389-
expect(assignedRuleIds).toHaveLength(3);
390-
expect(assignedRuleIds).toEqual(expect.arrayContaining([root.ruleId, child.ruleId, grandchild.ruleId]));
389+
expect(assignedRuleIds).toHaveLength(3); // grandchild, child, topLevelRule
390+
expect(assignedRuleIds).toEqual(expect.arrayContaining([topLevelRule.ruleId, child.ruleId, grandchild.ruleId]));
391391
});
392392

393-
it('Creates project rules for shared ancestors when adding a sibling sub-rule', async () => {
394-
const root = await RulesService.createRule(batman, 'T.1', 'Technical Rules', rulesetId, organization);
393+
it('Creates project rules for shared ancestors when adding a sibling child rule', async () => {
394+
const topLevelRule = await RulesService.createRule(batman, 'T.1', 'Technical Rules', rulesetId, organization);
395395
const child = await RulesService.createRule(
396396
batman,
397397
'T.1.1',
398398
'Vehicle Requirements',
399399
rulesetId,
400400
organization,
401-
root.ruleId
401+
topLevelRule.ruleId
402402
);
403403
const grandchild1 = await RulesService.createRule(batman, 'T.1.1.1', 'Wheels', rulesetId, organization, child.ruleId);
404404
const grandchild2 = await RulesService.createRule(batman, 'T.1.1.2', 'Brakes', rulesetId, organization, child.ruleId);
405405

406406
const project = await createTestProject(aquaman, orgId, undefined, carId);
407407

408408
await RulesService.createProjectRule(aquaman, organization, grandchild1.ruleId, project.projectId);
409-
// Adding a sibling must not error on the already-present parent/root and must not duplicate them.
409+
// adding sibling must not error or duplicate the already-present parent/root rules
410410
await RulesService.createProjectRule(aquaman, organization, grandchild2.ruleId, project.projectId);
411411

412412
const projectRules = await RulesService.getProjectRules(rulesetId, project.projectId, organization);
413413
const assignedRuleIds = projectRules.map((pr) => pr.rule.ruleId);
414-
expect(assignedRuleIds).toHaveLength(4);
414+
expect(assignedRuleIds).toHaveLength(4); // grandchild1, grandchild2, child, topLevelRule
415415
expect(assignedRuleIds).toEqual(
416-
expect.arrayContaining([root.ruleId, child.ruleId, grandchild1.ruleId, grandchild2.ruleId])
416+
expect.arrayContaining([topLevelRule.ruleId, child.ruleId, grandchild1.ruleId, grandchild2.ruleId])
417417
);
418418
});
419419

420-
it('does not assign descendants of the selected rule', async () => {
421-
const root = await RulesService.createRule(batman, 'T.1', 'Technical Rules', rulesetId, organization);
420+
it('Creating project rule does not assign descendants of the selected rule', async () => {
421+
const topLevelRule = await RulesService.createRule(batman, 'T.1', 'Technical Rules', rulesetId, organization);
422422
const child = await RulesService.createRule(
423423
batman,
424424
'T.1.1',
425425
'Vehicle Requirements',
426426
rulesetId,
427427
organization,
428-
root.ruleId
428+
topLevelRule.ruleId
429429
);
430430
await RulesService.createRule(batman, 'T.1.1.1', 'Wheels', rulesetId, organization, child.ruleId);
431431

@@ -435,43 +435,43 @@ describe('Create Rules Tests', () => {
435435

436436
const projectRules = await RulesService.getProjectRules(rulesetId, project.projectId, organization);
437437
const assignedRuleIds = projectRules.map((pr) => pr.rule.ruleId);
438-
expect(assignedRuleIds).toHaveLength(2);
439-
expect(assignedRuleIds).toEqual(expect.arrayContaining([root.ruleId, child.ruleId]));
438+
expect(assignedRuleIds).toHaveLength(2); // child and topLevelRule, not grandchild
439+
expect(assignedRuleIds).toEqual(expect.arrayContaining([topLevelRule.ruleId, child.ruleId]));
440440
});
441441

442-
it('does not assign deleted ancestors when assigning a nested sub-rule', async () => {
443-
const root = await RulesService.createRule(batman, 'T.1', 'Technical Rules', rulesetId, organization);
442+
it('Creating project rule is refused entirely when an ancestor has been deleted', async () => {
443+
const topLevelRule = await RulesService.createRule(batman, 'T.1', 'Technical Rules', rulesetId, organization);
444444
const child = await RulesService.createRule(
445445
batman,
446446
'T.1.1',
447447
'Vehicle Requirements',
448448
rulesetId,
449449
organization,
450-
root.ruleId
450+
topLevelRule.ruleId
451451
);
452452
const grandchild = await RulesService.createRule(batman, 'T.1.1.1', 'Wheels', rulesetId, organization, child.ruleId);
453453

454-
// Soft-delete the immediate parent directly so the grandchild remains assignable
454+
// soft-delete an ancestor so the chain to the top-level rule is broken
455455
await prisma.rule.update({
456456
where: { ruleId: child.ruleId },
457457
data: { dateDeleted: new Date(), deletedBy: { connect: { userId: batman.userId } } }
458458
});
459459

460460
const project = await createTestProject(aquaman, orgId, undefined, carId);
461461

462-
await RulesService.createProjectRule(aquaman, organization, grandchild.ruleId, project.projectId);
462+
// a broken chain means the grandchild could never display, so no rules are assigned to the project and an error is thrown
463+
await expect(
464+
async () => await RulesService.createProjectRule(aquaman, organization, grandchild.ruleId, project.projectId)
465+
).rejects.toThrow(new DeletedException('Rule', child.ruleId));
463466

464-
// The walk stops at the deleted ancestor, so neither it nor the root above it are assigned.
467+
// nothing should have been assigned (not the grandchild, the deleted parent, or the root)
465468
const projectRules = await RulesService.getProjectRules(rulesetId, project.projectId, organization);
466-
const assignedRuleIds = projectRules.map((pr) => pr.rule.ruleId);
467-
expect(assignedRuleIds).toHaveLength(1);
468-
expect(assignedRuleIds).toEqual([grandchild.ruleId]);
469+
expect(projectRules).toHaveLength(0);
469470

470-
// getProjectRules hides deleted rules, so assert directly that none was created for the deleted ancestor
471-
const deletedAncestorProjectRule = await prisma.project_Rule.findUnique({
472-
where: { ruleId_projectId: { ruleId: child.ruleId, projectId: project.projectId } }
471+
const grandchildProjectRule = await prisma.project_Rule.findUnique({
472+
where: { ruleId_projectId: { ruleId: grandchild.ruleId, projectId: project.projectId } }
473473
});
474-
expect(deletedAncestorProjectRule).toBeNull();
474+
expect(grandchildProjectRule).toBeNull();
475475
});
476476

477477
it('throws when the rule is already associated with the project', async () => {

0 commit comments

Comments
 (0)