Browse Source

fix(ecosystem): Preserve codeowners filters on update (#45810)

Scott Cooper 2 years ago
parent
commit
7c2e9034a3

+ 34 - 0
static/app/views/settings/project/projectOwnership/ownershipRulesTable.spec.tsx

@@ -104,6 +104,40 @@ describe('OwnershipRulesTable', () => {
     expect(screen.queryByText('mytag')).toBeInTheDocument();
   });
 
+  it('preserves selected teams when rules are updated', () => {
+    const rules: ParsedOwnershipRule[] = [
+      {
+        matcher: {pattern: 'filepath', type: 'path'},
+        owners: [{type: 'user', id: user1.id, name: user1.name}],
+      },
+      {
+        matcher: {pattern: 'anotherpath', type: 'path'},
+        owners: [{type: 'user', id: user2.id, name: user2.name}],
+      },
+    ];
+
+    const {rerender} = render(
+      <OwnershipRulesTable projectRules={rules} codeowners={[]} />
+    );
+
+    // Clear the filter
+    userEvent.click(screen.getByRole('button', {name: 'My Teams'}));
+    userEvent.click(screen.getByRole('button', {name: 'Clear'}));
+    expect(screen.getAllByText('path')).toHaveLength(2);
+
+    const newRules: ParsedOwnershipRule[] = [
+      ...rules,
+      {
+        matcher: {pattern: 'thirdpath', type: 'path'},
+        owners: [{type: 'user', id: user2.id, name: user2.name}],
+      },
+    ];
+
+    rerender(<OwnershipRulesTable projectRules={newRules} codeowners={[]} />);
+    expect(screen.getAllByText('path')).toHaveLength(3);
+    expect(screen.getByRole('button', {name: 'Everyone'})).toBeInTheDocument();
+  });
+
   it('should paginate results', () => {
     const owners: Actor[] = [{type: 'user', id: user1.id, name: user1.name}];
     const rules: ParsedOwnershipRule[] = Array(100)

+ 10 - 7
static/app/views/settings/project/projectOwnership/ownshipRulesTable.tsx

@@ -41,7 +41,7 @@ export function OwnershipRulesTable({
 }: OwnershipRulesTableProps) {
   const [search, setSearch] = useState<string>('');
   const [page, setPage] = useState<number>(0);
-  const [selectedActors, setSelectedActors] = useState<string[]>([]);
+  const [selectedActors, setSelectedActors] = useState<string[] | null>(null);
   const {teams} = useTeams({provideUserTeams: true});
 
   const combinedRules = useMemo(() => {
@@ -52,7 +52,7 @@ export function OwnershipRulesTable({
       }))
     );
 
-    return [...projectRules, ...codeownerRulesWithId];
+    return [...codeownerRulesWithId, ...projectRules];
   }, [projectRules, codeowners]);
 
   /**
@@ -91,10 +91,10 @@ export function OwnershipRulesTable({
   }, [allActors, teams]);
 
   useEffect(() => {
-    if (myTeams.length > 0) {
+    if (myTeams.length > 0 && selectedActors === null) {
       setSelectedActors(myTeams.map(actor => `${actor.type}:${actor.id}`));
     }
-  }, [myTeams]);
+  }, [myTeams, selectedActors]);
 
   /**
    * Rules chunked into pages
@@ -104,8 +104,10 @@ export function OwnershipRulesTable({
       rule =>
         // Filter by query
         (rule.matcher.type.includes(search) || rule.matcher.pattern.includes(search)) &&
-        // Filter by selected actors
-        (selectedActors.length === 0 ||
+        // Selected actors not set
+        (selectedActors === null ||
+          // Selected actors was cleared
+          selectedActors.length === 0 ||
           rule.owners.some(owner => selectedActors.includes(`${owner.type}:${owner.id}`)))
     );
 
@@ -137,9 +139,10 @@ export function OwnershipRulesTable({
       <SearchAndSelectorWrapper>
         <OwnershipOwnerFilter
           actors={allActors}
-          selectedTeams={selectedActors}
+          selectedTeams={selectedActors ?? []}
           handleChangeFilter={handleChangeFilter}
           isMyTeams={
+            !!selectedActors &&
             selectedActors.length > 0 &&
             isEqual(
               selectedActors,