#4315 editing rule codes

src/backend/src/routes/rules.routes.ts

@@ -13,7 +13,7 @@ rulesRouter.get('/ruleset/:rulesetId', RulesController.getRulesetById);
 rulesRouter.post(
   '/rule/create',
   nonEmptyString(body('ruleCode')),
-  nonEmptyString(body('ruleContent')),
+  body('ruleContent').isString(),
   nonEmptyString(body('rulesetId')),
   body('parentRuleId').optional().isString(),
   body('referencedRules').optional().isArray(),
@@ -25,8 +25,8 @@ rulesRouter.post(
 );
 rulesRouter.post(
   '/rule/:ruleId/edit',
-  nonEmptyString(body('ruleContent')),
-  body('ruleCode').optional().isString(),
+  body('ruleContent').isString(),
+  nonEmptyString(body('ruleCode').optional()),
   body('imageFileIds').optional().isArray(),
   body('imageFileIds.*').optional().isString(),
   body('parentRuleId').optional().isString(),

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

@@ -96,6 +96,26 @@ export default class RulesService {
     return rulesetTransformer(ruleset);
   }
 
+  /**
+   * Throws if a rule with the given code already exists in the given ruleset
+   * @param rulesetId The ruleset to check for an existing rule code
+   * @param ruleCode The trimmed rule code to check
+   */
+  private static async assertRuleCodeAvailable(rulesetId: string, ruleCode: string) {
+    const existingRule = await prisma.rule.findUnique({
+      where: {
+        rulesetId_ruleCode: {
+          rulesetId,
+          ruleCode
+        }
+      }
+    });
+
+    if (existingRule) {
+      throw new HttpException(400, `Rule with code ${ruleCode} already exists in this ruleset`);
+    }
+  }
+
   /**
    * Creates a new rule in the database
    *
@@ -148,19 +168,10 @@ export default class RulesService {
       throw new AccessDeniedException('Cannot create rule in a ruleset from another organization');
     }
 
-    // Check for duplicate rule code within the same ruleset
-    const existingRule = await prisma.rule.findUnique({
-      where: {
-        rulesetId_ruleCode: {
-          rulesetId,
-          ruleCode
-        }
-      }
-    });
+    ruleCode = ruleCode.trim();
 
-    if (existingRule) {
-      throw new HttpException(400, `Rule with code ${ruleCode} already exists in this ruleset`);
-    }
+    // Check for duplicate rule code within the same ruleset
+    await RulesService.assertRuleCodeAvailable(rulesetId, ruleCode);
 
     // Verify parent rule exists if provided
     if (parentRuleId) {
@@ -507,6 +518,18 @@ export default class RulesService {
     if (currentRule.ruleset?.car?.wbsElement?.organizationId !== organization.organizationId)
       throw new InvalidOrganizationException('Rule');
 
+    if (ruleCode !== undefined) {
+      ruleCode = ruleCode.trim();
+
+      if (ruleCode === '') {
+        throw new HttpException(400, 'Rule code cannot be empty');
+      }
+
+      if (ruleCode !== currentRule.ruleCode) {
+        await RulesService.assertRuleCodeAvailable(currentRule.rulesetId, ruleCode);
+      }
+    }
+
     if (parentRuleId) {
       const parentRule = await prisma.rule.findUnique({
         where: { ruleId: parentRuleId }
@@ -519,6 +542,10 @@ export default class RulesService {
       if (parentRule.dateDeleted) {
         throw new DeletedException('Parent Rule', parentRuleId);
       }
+
+      if (parentRule.rulesetId !== currentRule.rulesetId) {
+        throw new HttpException(400, 'Parent rule must be in the same ruleset');
+      }
     }
 
     const updatedRule = await prisma.rule.update({

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

@@ -232,6 +232,21 @@ describe('Create Rules Tests', () => {
       ).rejects.toThrow(new HttpException(400, 'Parent rule must be in the same ruleset'));
     });
 
+    // this is allowed but with warning in case a parent code needs to be updated which won't break all existing children
+    it('allows a child rule code that does not start with the parent rule code', async () => {
+      const parentRule = await RulesService.createRule(batman, 'T.1', 'Parent', rulesetId, organization);
+
+      const childRule = await RulesService.createRule(superman, 'TH.1', 'Child', rulesetId, organization, parentRule.ruleId);
+
+      expect(childRule.ruleCode).toBe('TH.1');
+      expect(childRule.parentRule?.ruleId).toBe(parentRule.ruleId);
+    });
+
+    it('successfully creates a rule with blank content', async () => {
+      const rule = await RulesService.createRule(batman, 'T.9.1', '', rulesetId, organization);
+      expect(rule.ruleContent).toBe('');
+    });
+
     it('fails when referenced rule does not exist', async () => {
       await expect(
         RulesService.createRule(batman, 'T.1.1', 'Some rule', rulesetId, organization, undefined, ['fake-rule-id'])
@@ -1198,6 +1213,123 @@ describe('Rule Tests', () => {
 
       expect(updatedRule.ruleContent).toEqual('BRAND NEW RULE CONTENT');
     });
+
+    it('Succeeds and edits a rule to have blank content', async () => {
+      const car = await createUniqueCar(orgId);
+      const { leafRule1 } = await setupRules(car);
+      const updatedRule = await RulesService.editRule(
+        admin,
+        '',
+        leafRule1.ruleId,
+        leafRule1.ruleCode,
+        leafRule1.imageFileIds,
+        organization
+      );
+
+      expect(updatedRule.ruleContent).toEqual('');
+    });
+
+    it('Fails when new rule code is blank', async () => {
+      const car = await createUniqueCar(orgId);
+      const { leafRule1 } = await setupRules(car);
+
+      await expect(
+        RulesService.editRule(admin, leafRule1.ruleContent, leafRule1.ruleId, '', leafRule1.imageFileIds, organization)
+      ).rejects.toThrow(new HttpException(400, 'Rule code cannot be empty'));
+    });
+
+    it('Succeeds and changes a rule code that still satisfies the parent prefix', async () => {
+      const car = await createUniqueCar(orgId);
+      const { leafRule1 } = await setupRules(car);
+
+      const updatedRule = await RulesService.editRule(
+        admin,
+        leafRule1.ruleContent,
+        leafRule1.ruleId,
+        'T99',
+        leafRule1.imageFileIds,
+        organization
+      );
+
+      expect(updatedRule.ruleCode).toEqual('T99');
+    });
+
+    it('Fails when new rule code duplicates another rule in the same ruleset', async () => {
+      const car = await createUniqueCar(orgId);
+      const { leafRule1, leafRule2 } = await setupRules(car);
+
+      await expect(
+        RulesService.editRule(
+          admin,
+          leafRule1.ruleContent,
+          leafRule1.ruleId,
+          leafRule2.ruleCode,
+          leafRule1.imageFileIds,
+          organization
+        )
+      ).rejects.toThrow(new HttpException(400, `Rule with code ${leafRule2.ruleCode} already exists in this ruleset`));
+    });
+
+    it('Fails when new rule code duplicates another rule code padded with whitespace', async () => {
+      const car = await createUniqueCar(orgId);
+      const { leafRule1, leafRule2 } = await setupRules(car);
+
+      await expect(
+        RulesService.editRule(
+          admin,
+          leafRule1.ruleContent,
+          leafRule1.ruleId,
+          `  ${leafRule2.ruleCode}  `,
+          leafRule1.imageFileIds,
+          organization
+        )
+      ).rejects.toThrow(new HttpException(400, `Rule with code ${leafRule2.ruleCode} already exists in this ruleset`));
+    });
+
+    it('Fails when parent rule is in a different ruleset', async () => {
+      const car = await createUniqueCar(orgId);
+      const { ruleset2, leafRule1 } = await setupRules(car);
+
+      const otherRulesetRule = await prisma.rule.create({
+        data: {
+          ruleCode: 'X1',
+          ruleContent: 'Rule in a different ruleset',
+          imageFileIds: [],
+          dateCreated: new Date(),
+          ruleset: { connect: { rulesetId: ruleset2.rulesetId } },
+          createdBy: { connect: { userId: admin.userId } }
+        }
+      });
+
+      await expect(
+        RulesService.editRule(
+          admin,
+          leafRule1.ruleContent,
+          leafRule1.ruleId,
+          leafRule1.ruleCode,
+          leafRule1.imageFileIds,
+          organization,
+          otherRulesetRule.ruleId
+        )
+      ).rejects.toThrow(new HttpException(400, 'Parent rule must be in the same ruleset'));
+    });
+
+    it('Allows a new rule code that does not start with the existing parent rule code', async () => {
+      const car = await createUniqueCar(orgId);
+      const { leafRule2 } = await setupRules(car); // leafRule2's parent code is 'T'
+
+      const updatedRule = await RulesService.editRule(
+        admin,
+        leafRule2.ruleContent,
+        leafRule2.ruleId,
+        'X2.1',
+        leafRule2.imageFileIds,
+        organization
+      );
+
+      expect(updatedRule.parentRule?.ruleCode).not.toEqual('X2');
+      expect(updatedRule.ruleCode).toEqual('X2.1');
+    });
   });
 
   describe('Delete Ruleset', () => {

src/frontend/src/apis/rules.api.ts

@@ -160,12 +160,13 @@ export const deleteRule = (ruleId: string) => {
 };
 
 /**
- * Edits a rule's content
+ * Edits a rule's content and/or code
  * @param ruleId - The ID of the rule to edit
  * @param ruleContent - The new content for the rule
+ * @param ruleCode - The new code for the rule (optional, keeps existing if not provided)
  */
-export const editRule = (ruleId: string, ruleContent: string) => {
-  return axios.post<SharedRule>(apiUrls.rulesEdit(ruleId), { ruleContent });
+export const editRule = (ruleId: string, ruleContent: string, ruleCode?: string) => {
+  return axios.post<SharedRule>(apiUrls.rulesEdit(ruleId), { ruleContent, ruleCode });
 };
 
 /**

src/frontend/src/hooks/rules.hooks.ts

@@ -413,10 +413,10 @@ export const useEditRule = () => {
   const queryClient = useQueryClient();
   const toast = useToast();
 
-  return useMutation<SharedRule, Error, { ruleId: string; ruleContent: string }>(
+  return useMutation<SharedRule, Error, { ruleId: string; ruleContent: string; ruleCode?: string }>(
     ['rules', 'edit'],
-    async ({ ruleId, ruleContent }) => {
-      const { data } = await editRule(ruleId, ruleContent);
+    async ({ ruleId, ruleContent, ruleCode }) => {
+      const { data } = await editRule(ruleId, ruleContent, ruleCode);
       return data;
     },
     {

src/frontend/src/pages/RulesPage/AssignRulesTab.tsx

@@ -15,7 +15,7 @@ import {
   Typography,
   useTheme
 } from '@mui/material';
-import { useState, useEffect } from 'react';
+import { useState, useEffect, useRef } from 'react';
 import { Rule, TeamPreview } from 'shared';
 import { useAllTeams } from '../../hooks/teams.hooks';
 import LoadingIndicator from '../../components/LoadingIndicator';
@@ -92,7 +92,6 @@ const AssignRulesTab: React.FC<AssignRulesTabProps> = ({ rules }) => {
   const [selectedTeamId, setSelectedTeamId] = useState<string | null>(null);
   const [assignments, setAssignments] = useState<Set<string>>(new Set());
   const [originalAssignments, setOriginalAssignments] = useState<Set<string>>(new Set());
-  const [isInitialized, setIsInitialized] = useState(false);
   // unassign impact for the amount of project rules that would be deleted by this action, used to warn the user before saving
   const [pendingToggles, setPendingToggles] = useState<Array<{ ruleId: string; teamId: string }> | null>(null);
   const [unassignImpact, setUnassignImpact] = useState<{ ruleCount: number; projectCount: number }>({
@@ -103,28 +102,32 @@ const AssignRulesTab: React.FC<AssignRulesTabProps> = ({ rules }) => {
   const { data: teams, isLoading: teamsLoading, isError: teamsError, error: teamsErrorData } = useAllTeams();
   const { mutate: bulkToggle, isLoading: isSaving } = useBulkToggleRuleTeam();
 
-  // Load initial team assignments from rule data
-  useEffect(() => {
-    if (isInitialized || !teams || teams.length === 0) return;
+  // background refetches ensure current toggles remain
+  const hasPendingChangesRef = useRef(false);
 
-    const initialAssignments = new Set<string>();
-    rules.forEach((rule) => {
-      rule.teams?.forEach((team) => {
-        initialAssignments.add(`${team.teamId}:${rule.ruleId}`);
+  // Reload when rule data changes, unless the user has unsaved toggles pending
+  useEffect(() => {
+    if (!teams || teams.length === 0) return;
+
+    if (!hasPendingChangesRef.current) {
+      const initialAssignments = new Set<string>();
+      rules.forEach((rule) => {
+        rule.teams?.forEach((team) => {
+          initialAssignments.add(`${team.teamId}:${rule.ruleId}`);
+        });
       });
-    });
-
-    setOriginalAssignments(initialAssignments);
-    setAssignments(new Set(initialAssignments));
 
-    // Pre-select the team passed in via query param (e.g. when navigating from a project's rules tab)
-    const teamIdParam = new URLSearchParams(location.search).get('teamId');
-    if (teamIdParam && teams.some((team) => team.teamId === teamIdParam)) {
-      setSelectedTeamId(teamIdParam);
+      setOriginalAssignments(initialAssignments);
+      setAssignments(new Set(initialAssignments));
     }
 
-    setIsInitialized(true);
-  }, [rules, teams, isInitialized, location.search]);
+    if (!selectedTeamId) {
+      const teamIdParam = new URLSearchParams(location.search).get('teamId');
+      if (teamIdParam && teams.some((team) => team.teamId === teamIdParam)) {
+        setSelectedTeamId(teamIdParam);
+      }
+    }
+  }, [rules, teams, location.search, selectedTeamId]);
 
   const handleTeamSelect = (teamId: string) => setSelectedTeamId(teamId);
 
@@ -211,6 +214,7 @@ const AssignRulesTab: React.FC<AssignRulesTabProps> = ({ rules }) => {
       getAncestorIds(ruleId, rules).forEach((id) => newAssignments.add(teamAssignmentKey(id)));
     }
 
+    hasPendingChangesRef.current = true;
     setAssignments(newAssignments);
   };
 
@@ -246,6 +250,7 @@ const AssignRulesTab: React.FC<AssignRulesTabProps> = ({ rules }) => {
   const executeToggles = (toggles: Array<{ ruleId: string; teamId: string }>) => {
     bulkToggle(toggles, {
       onSuccess: () => {
+        hasPendingChangesRef.current = false;
         history.push(routes.RULESET_EDIT.replace(':rulesetId', rulesetId));
       },
       onSettled: () => {

src/frontend/src/pages/RulesPage/RuleRow.tsx

@@ -14,7 +14,13 @@ interface RuleRowProps {
   rule: Rule;
   allRules?: Rule[];
   level?: number;
-  leftContent?: (rule: Rule, level: number, isExpanded: boolean, hasSubRules: boolean) => React.ReactNode;
+  leftContent?: (
+    rule: Rule,
+    level: number,
+    isExpanded: boolean,
+    hasSubRules: boolean,
+    toggleExpand: () => void
+  ) => React.ReactNode;
   middleContent?: (rule: Rule, level: number) => React.ReactNode;
   rightContent: (rule: Rule, level: number) => React.ReactNode;
   backgroundColor: string | ((rule: Rule) => string);
@@ -203,7 +209,7 @@ const RuleRow: React.FC<RuleRowProps> = ({
               whiteSpace: 'normal'
             }}
           >
-            {leftContent ? leftContent(rule, level, isExpanded, hasSubRules) : defaultLeftContent}
+            {leftContent ? leftContent(rule, level, isExpanded, hasSubRules, toggleExpand) : defaultLeftContent}
           </TableCell>
         ) : (
           <>
@@ -222,7 +228,7 @@ const RuleRow: React.FC<RuleRowProps> = ({
                 width: leftWidth
               }}
             >
-              {leftContent ? leftContent(rule, level, isExpanded, hasSubRules) : defaultLeftContent}
+              {leftContent ? leftContent(rule, level, isExpanded, hasSubRules, toggleExpand) : defaultLeftContent}
             </TableCell>
             <TableCell
               align="left"