Fixes #38901 - Refactor sync status page with React

[jeremylenz]

What are the changes introduced in this pull request?

This PR redesigns the sync status page using React and improves its UI. Key user-facing changes:

• Backwards compatibility: Old /katello/sync_management URL now redirects to new /sync_management URL
• Improved UI: Removed gray border from sync status table for cleaner appearance that matches other pages
• Better reliability: Added comprehensive test coverage with 9 new tests

Technical implementation:

• Migrated page from ERB to React
• Added route redirect for backwards compatibility
• All ESLint and Rubocop checks passing

Considerations taken when implementing this change?

• Used TableIndexPage from Foreman for consistent UI patterns
• Maintained backwards compatibility by redirecting the old URL route
• Followed existing Katello patterns for React component structure and testing
• Added fireEvent to existing tests for consistency
• Ensured proper test coverage with both component and integration tests

What are the testing steps for this pull request?

1. Verify the old URL redirects: Navigate to /katello/sync_management and confirm it redirects to /sync_management
2. Verify UI appearance: Check that the sync status table has no gray border and matches the styling of other Katello pages
3. Test functionality: Ensure sync status data displays correctly and all interactive elements work
4. Verify tests: All 22 JavaScript tests and 4 Ruby tests passing

Fixes #38901

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• Remove the large commented‐out block for the old sync_management routes (or clearly document your rollback procedure) to avoid clutter and dead code.
• SyncStatusPage is very heavy on logic—consider extracting the polling mechanism and tree traversal (ancestor/ID collection, row building) into custom hooks or shared utilities to improve readability and reduce duplication.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Remove the large commented‐out block for the old sync_management routes (or clearly document your rollback procedure) to avoid clutter and dead code.
- SyncStatusPage is very heavy on logic—consider extracting the polling mechanism and tree traversal (ancestor/ID collection, row building) into custom hooks or shared utilities to improve readability and reduce duplication.

## Individual Comments

### Comment 1
<location> `webpack/scenes/SyncStatus/components/SyncStatusTable.js:31-37` </location>
<code_context>
+  const buildTreeRows = useMemo(() => {
+    const rows = [];
+
+    const addRow = (node, level, parent, posinset, isHidden) => {
+      const nodeId = `${node.type}-${node.id}`;
+      const hasChildren = (node.children && node.children.length > 0) ||
+                          (node.repos && node.repos.length > 0);
+      const isExpanded = expandedNodeIds.includes(nodeId);
+
+      rows.push({
+        ...node,
+        nodeId,
</code_context>

<issue_to_address>
**suggestion:** Tree row aria attributes may not reflect actual set size for accessibility.

Recalculate aria-setsize after filtering so it matches the number of visible siblings, ensuring correct interpretation by accessibility tools.

Suggested implementation:

```javascript
    const addRow = (node, level, parent, posinset, isHidden, ariaSetSize) => {
      const nodeId = `${node.type}-${node.id}`;
      const hasChildren = (node.children && node.children.length > 0) ||
                          (node.repos && node.repos.length > 0);
      const isExpanded = expandedNodeIds.includes(nodeId);

      rows.push({
        ...node,
        nodeId,
        level,
        parent,
        posinset,
        isHidden,
        hasChildren,
        isExpanded,
        ariaSetSize,
      });

      if (hasChildren && !isHidden) {
        const childrenToRender = node.children || [];

```

```javascript
        const childrenToRender = node.children || [];
        // Filter children to only visible ones
        const visibleChildren = childrenToRender.filter(child => {
          // If showActiveOnly is true, filter out inactive nodes
          if (showActiveOnly && !child.isActive) return false;
          // If child is hidden, filter out
          if (child.isHidden) return false;
          return true;
        });
        visibleChildren.forEach((child, idx) => {
          addRow(
            child,
            level + 1,
            nodeId,
            idx + 1,
            false,
            visibleChildren.length // Pass correct aria-setsize
          );
        });

        // If there are repos, treat them as children as well
        if (node.repos && node.repos.length > 0) {
          const visibleRepos = node.repos.filter(repo => {
            if (showActiveOnly && !repo.isActive) return false;
            if (repo.isHidden) return false;
            return true;
          });
          visibleRepos.forEach((repo, idx) => {
            addRow(
              repo,
              level + 1,
              nodeId,
              idx + 1,
              false,
              visibleRepos.length // Pass correct aria-setsize
            );
          });
        }

```

```javascript
    const buildTreeRows = useMemo(() => {
        const rows = [];

        // Helper to get visible root nodes
        const getVisibleRoots = (nodes) => {
          return nodes.filter(node => {
            if (showActiveOnly && !node.isActive) return false;
            if (node.isHidden) return false;
            return true;
          });
        };

        // Start with root nodes
        const rootNodes = getVisibleRoots(/* your root nodes source here, e.g. data.nodes */);
        rootNodes.forEach((node, idx) => {
          addRow(
            node,
            1,
            null,
            idx + 1,
            false,
            rootNodes.length // Pass correct aria-setsize for root nodes
          );
        });

        return rows;
    }, [expandedNodeIds, showActiveOnly]);

```

You will need to:
1. Replace `/* your root nodes source here, e.g. data.nodes */` with the actual array of root nodes in your tree.
2. Ensure that wherever you render the tree rows, you use the new `ariaSetSize` property for the `aria-setsize` attribute.
3. If you have other places where children are rendered, apply the same visible filtering logic.
</issue_to_address>

### Comment 2
<location> `webpack/scenes/SyncStatus/components/SyncStatusTable.js:66-75` </location>
<code_context>
+
+  // Filter rows based on active only setting
+  const visibleRows = useMemo(() => {
+    if (!showActiveOnly) return buildTreeRows;
+
+    return buildTreeRows.filter((row) => {
+      if (row.type !== 'repo') return true;
+      const status = repoStatuses[row.id];
</code_context>

<issue_to_address>
**suggestion:** Filtering for active only may leave parent nodes with no visible children.

Consider updating the logic to hide parent nodes when all their child repos are filtered out, ensuring the UI only displays parents with visible children.

```suggestion
  // Filter rows based on active only setting
  const visibleRows = useMemo(() => {
    if (!showActiveOnly) return buildTreeRows;

    // Build a map of rowId to row for quick lookup
    const rowMap = {};
    buildTreeRows.forEach(row => {
      rowMap[row.id] = row;
    });

    // Build a map of parentId to its children
    const parentToChildren = {};
    buildTreeRows.forEach(row => {
      if (row.parentId) {
        if (!parentToChildren[row.parentId]) parentToChildren[row.parentId] = [];
        parentToChildren[row.parentId].push(row);
      }
    });

    // Helper to determine if a row (parent) has any visible children
    function hasVisibleChildren(row) {
      const children = parentToChildren[row.id] || [];
      return children.some(child => isRowVisible(child));
    }

    // Helper to determine if a row is visible
    function isRowVisible(row) {
      if (row.type === 'repo') {
        const status = repoStatuses[row.id];
        return status?.is_running;
      }
      // For parent nodes, visible if any child is visible
      return hasVisibleChildren(row);
    }

    // Only include rows that are visible
    return buildTreeRows.filter(row => isRowVisible(row));
  }, [buildTreeRows, showActiveOnly, repoStatuses]);
```
</issue_to_address>

### Comment 3
<location> `webpack/scenes/SyncStatus/components/SyncResultCell.js:48-49` </location>
<code_context>
+
+  const taskUrl = syncId ? foremanUrl(`/foreman_tasks/tasks/${syncId}`) : null;
+
+  const labelContent = (
+    <Label color={color} icon={icon}>
+      {taskUrl ? (
+        <a href={taskUrl} target="_blank" rel="noopener noreferrer">
</code_context>

<issue_to_address>
**issue (bug_risk):** Label color 'grey' is not a valid PatternFly color.

Use 'default' or another supported PatternFly color instead of 'grey' to prevent rendering problems.
</issue_to_address>

### Comment 4
<location> `webpack/scenes/SyncStatus/components/SyncProgressCell.js:18` </location>
<code_context>
+    return null;
+  }
+
+  const progressValue = progress?.progress || 0;
+
+  return (
</code_context>

<issue_to_address>
**suggestion:** Progress value may exceed 100 or be negative if API returns unexpected data.

Clamp progressValue to the 0–100 range to prevent display problems from unexpected API values.

```suggestion
  const progressValueRaw = progress?.progress || 0;
  const progressValue = Math.max(0, Math.min(100, progressValueRaw));
```
</issue_to_address>

### Comment 5
<location> `webpack/scenes/SyncStatus/SyncStatusPage.js:138-103` </location>
<code_context>
+  // Get all repository IDs from the tree
+  const getAllRepoIds = useCallback(() => {
+    const repoIds = [];
+    const traverse = (nodes) => {
+      nodes.forEach((node) => {
+        if (node.type === 'repo') {
+          repoIds.push(node.id);
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Traversing both children and repos may result in duplicate repo IDs.

Using a Set instead of an array will prevent duplicate repo IDs when aggregating from both sources.
</issue_to_address>

### Comment 6
<location> `webpack/scenes/SyncStatus/components/__tests__/SyncStatusTable.test.js:70-77` </location>
<code_context>
+    expect(screen.getByText('Test Product')).toBeInTheDocument();
+  });
+
+  it('expands rows when clicked', () => {
+    render(<SyncStatusTable {...mockProps} />);
+
+    const expandButtons = screen.getAllByRole('button', { name: /expand row/i });
+    fireEvent.click(expandButtons[0]);
+
+    expect(mockProps.setExpandedNodeIds).toHaveBeenCalled();
+  });
+
</code_context>

<issue_to_address>
**suggestion (testing):** Missing test for collapsing expanded rows.

Please add a test to confirm that collapsing an expanded row hides its child rows and updates expandedNodeIds as expected.

```suggestion
  it('expands rows when clicked', () => {
    render(<SyncStatusTable {...mockProps} />);

    const expandButtons = screen.getAllByRole('button', { name: /expand row/i });
    fireEvent.click(expandButtons[0]);

    expect(mockProps.setExpandedNodeIds).toHaveBeenCalled();
  });

  it('collapses expanded rows when clicked again', () => {
    render(<SyncStatusTable {...mockProps} />);

    // Expand the first row
    const expandButtons = screen.getAllByRole('button', { name: /expand row/i });
    fireEvent.click(expandButtons[0]);

    // Child row should be visible
    expect(screen.getByText('Test Content')).toBeInTheDocument();

    // Collapse the row
    fireEvent.click(expandButtons[0]);

    // Child row should not be visible
    expect(screen.queryByText('Test Content')).not.toBeInTheDocument();

    // setExpandedNodeIds should be called again for collapse
    expect(mockProps.setExpandedNodeIds).toHaveBeenCalledTimes(2);
  });
```
</issue_to_address>

### Comment 7
<location> `webpack/scenes/SyncStatus/components/__tests__/SyncStatusTable.test.js:79-88` </location>
<code_context>
+    expect(mockProps.setExpandedNodeIds).toHaveBeenCalled();
+  });
+
+  it('calls onSelectRepo when checkbox is clicked', () => {
+    const propsWithExpandedNodes = {
+      ...mockProps,
+      expandedNodeIds: ['product-1', 'product_content-101'],
+    };
+
+    render(<SyncStatusTable {...propsWithExpandedNodes} />);
+
+    // Find the checkbox for the repository
+    const checkboxes = screen.getAllByRole('checkbox');
+    // eslint-disable-next-line promise/prefer-await-to-callbacks
+    const repoCheckbox = checkboxes.find(cb =>
+      cb.getAttribute('aria-label')?.includes('Test Repository'));
+
+    if (repoCheckbox) {
+      fireEvent.click(repoCheckbox);
+      expect(mockProps.onSelectRepo).toHaveBeenCalledWith(1);
+    }
+  });
+
</code_context>

<issue_to_address>
**suggestion (testing):** Test does not cover selecting multiple repositories.

Please add a test to verify that selecting multiple repositories updates selectedRepoIds and the UI as expected.

Suggested implementation:

```javascript
  it('calls onSelectRepo when checkbox is clicked', () => {
    const propsWithExpandedNodes = {
      ...mockProps,
      expandedNodeIds: ['product-1', 'product_content-101'],
    };

    render(<SyncStatusTable {...propsWithExpandedNodes} />);

    // Find the checkbox for the repository
    const checkboxes = screen.getAllByRole('checkbox');
    // eslint-disable-next-line promise/prefer-await-to-callbacks
    const repoCheckbox = checkboxes.find(cb =>
      cb.getAttribute('aria-label')?.includes('Test Repository'));

    if (repoCheckbox) {
      fireEvent.click(repoCheckbox);
      expect(mockProps.onSelectRepo).toHaveBeenCalledWith(1);
    }
  });

  it('selects multiple repositories and updates selectedRepoIds and UI', () => {
    const propsWithMultipleRepos = {
      ...mockProps,
      expandedNodeIds: ['product-1', 'product_content-101', 'product-2', 'product_content-201'],
      selectedRepoIds: [],
    };

    render(<SyncStatusTable {...propsWithMultipleRepos} />);

    // Find all repository checkboxes
    const checkboxes = screen.getAllByRole('checkbox');
    // Find two repo checkboxes by their aria-labels
    const repoCheckbox1 = checkboxes.find(cb =>
      cb.getAttribute('aria-label')?.includes('Test Repository'));
    const repoCheckbox2 = checkboxes.find(cb =>
      cb.getAttribute('aria-label')?.includes('Another Repository'));

    // Select both repositories
    if (repoCheckbox1 && repoCheckbox2) {
      fireEvent.click(repoCheckbox1);
      fireEvent.click(repoCheckbox2);

      // Expect onSelectRepo to have been called with both repo ids
      expect(mockProps.onSelectRepo).toHaveBeenCalledWith(1);
      expect(mockProps.onSelectRepo).toHaveBeenCalledWith(2);

      // Optionally, check that both checkboxes are now checked
      expect(repoCheckbox1.checked).toBe(true);
      expect(repoCheckbox2.checked).toBe(true);
    }
  });

```

- Ensure that your mock data (`mockProducts`) includes at least two repositories with distinct names and IDs (e.g., "Test Repository" and "Another Repository").
- If `selectedRepoIds` is managed by state in the parent, you may need to mock its update or check the effect on the UI.
- If the checkbox checked state is controlled by `selectedRepoIds`, make sure the test setup and assertions reflect this.
</issue_to_address>

### Comment 8
<location> `webpack/scenes/SyncStatus/components/__tests__/SyncProgressCell.test.js:5` </location>
<code_context>
+module Katello
+  class Api::V2::SyncStatusController < Api::V2::ApiController
+    include SyncManagementHelper::RepoMethods
+
+    before_action :find_optional_organization, :only => [:index, :poll, :sync]
+    before_action :find_repository, :only => [:destroy]
</code_context>

<issue_to_address>
**issue (testing):** No tests present for SyncProgressCell component.

Add unit tests to cover rendering during syncing, cancel button functionality, and non-rendering when not syncing.
</issue_to_address>

### Comment 9
<location> `webpack/scenes/SyncStatus/components/__tests__/SyncResultCell.test.js:9` </location>
<code_context>
+module Katello
+  class Api::V2::SyncStatusController < Api::V2::ApiController
+    include SyncManagementHelper::RepoMethods
+
+    before_action :find_optional_organization, :only => [:index, :poll, :sync]
+    before_action :find_repository, :only => [:destroy]
</code_context>

<issue_to_address>
**issue (testing):** No tests present for SyncResultCell component.

Add unit tests for SyncResultCell to cover all sync states, with and without errorDetails, and verify rendering of icons, labels, and tooltips.
</issue_to_address>

### Comment 10
<location> `webpack/scenes/SyncStatus/__tests__/SyncStatusPage.test.js:37-46` </location>
<code_context>
+  },
+};
+
+test('Can call API for sync status and show on screen on page load', async () => {
+  const scope = nockInstance
+    .get(syncStatusPath)
+    .query(true)
+    .reply(200, mockSyncStatusData);
+
+  const { queryByText } = renderWithRedux(<SyncStatusPage />, renderOptions);
+
+  // Initially shouldn't show the product
+  expect(queryByText('Test Product')).toBeNull();
+
+  // Wait for data to load
+  await patientlyWaitFor(() => {
+    expect(queryByText('Sync Status')).toBeInTheDocument();
+    expect(queryByText('Test Product')).toBeInTheDocument();
+  });
+
+  assertNockRequest(scope);
+});
+
</code_context>

<issue_to_address>
**suggestion (testing):** Missing test for error handling on API failure.

Add a test to confirm that SyncStatusPage displays an appropriate error message or fallback UI when the API call fails.

Suggested implementation:

```javascript
test('Can call API for sync status and show on screen on page load', async () => {
  const scope = nockInstance
    .get(syncStatusPath)
    .query(true)
    .reply(200, mockSyncStatusData);

  const { queryByText } = renderWithRedux(<SyncStatusPage />, renderOptions);

  // Initially shouldn't show the product
  expect(queryByText('Test Product')).toBeNull();

  // Wait for data to load
  await patientlyWaitFor(() => {
    expect(queryByText('Sync Status')).toBeInTheDocument();
    expect(queryByText('Test Product')).toBeInTheDocument();
  });

  assertNockRequest(scope);
});

test('Displays error message when API call fails', async () => {
  const scope = nockInstance
    .get(syncStatusPath)
    .query(true)
    .reply(500, { message: 'Internal Server Error' });

  const { queryByText } = renderWithRedux(<SyncStatusPage />, renderOptions);

  // Wait for error message to appear
  await patientlyWaitFor(() => {
    expect(queryByText(/error/i)).toBeInTheDocument();
  });

  assertNockRequest(scope);
});

```

- Ensure that `SyncStatusPage` actually displays an error message containing the word "error" when the API call fails. If it uses a different message or UI, adjust the test's `expect(queryByText(/error/i))` to match the actual error text or fallback UI.
</issue_to_address>

### Comment 11
<location> `webpack/scenes/SyncStatus/__tests__/SyncStatusPage.test.js:57-54` </location>
<code_context>
+  assertNockRequest(scope);
+});
+
+test('Displays toolbar with action buttons', async () => {
+  const scope = nockInstance
+    .get(syncStatusPath)
+    .query(true)
+    .reply(200, mockSyncStatusData);
+
+  const { queryByText } = renderWithRedux(<SyncStatusPage />, renderOptions);
+
+  await patientlyWaitFor(() => {
+    expect(queryByText('Expand All')).toBeInTheDocument();
+    expect(queryByText('Collapse All')).toBeInTheDocument();
+    expect(queryByText('Select All')).toBeInTheDocument();
+    expect(queryByText('Select None')).toBeInTheDocument();
+    expect(queryByText('Synchronize Now')).toBeInTheDocument();
+  });
+
+  assertNockRequest(scope);
+});
+
</code_context>

<issue_to_address>
**suggestion (testing):** Missing test for user interaction with toolbar actions.

Please add tests that simulate clicking each toolbar button and verify the expected actions are triggered.

Suggested implementation:

```javascript
test('Displays toolbar with action buttons', async () => {
  const scope = nockInstance
    .get(syncStatusPath)
    .query(true)
    .reply(200, mockSyncStatusData);

  const { queryByText } = renderWithRedux(<SyncStatusPage />, renderOptions);

  await patientlyWaitFor(() => {
    expect(queryByText('Expand All')).toBeInTheDocument();
    expect(queryByText('Collapse All')).toBeInTheDocument();
    expect(queryByText('Select All')).toBeInTheDocument();
    expect(queryByText('Select None')).toBeInTheDocument();
    expect(queryByText('Synchronize Now')).toBeInTheDocument();
  });

  assertNockRequest(scope);
});

test('Toolbar actions: clicking each button triggers expected behavior', async () => {
  const scope = nockInstance
    .get(syncStatusPath)
    .query(true)
    .reply(200, mockSyncStatusData);

  const { getByText, queryByText } = renderWithRedux(<SyncStatusPage />, renderOptions);

  await patientlyWaitFor(() => {
    expect(queryByText('Expand All')).toBeInTheDocument();
    expect(queryByText('Collapse All')).toBeInTheDocument();
    expect(queryByText('Select All')).toBeInTheDocument();
    expect(queryByText('Select None')).toBeInTheDocument();
    expect(queryByText('Synchronize Now')).toBeInTheDocument();
  });

  // Expand All
  fireEvent.click(getByText('Expand All'));
  // Add assertion for expanded state, e.g.:
  // expect(...).toHaveClass('expanded'); // Adjust to your UI

  // Collapse All
  fireEvent.click(getByText('Collapse All'));
  // Add assertion for collapsed state, e.g.:
  // expect(...).not.toHaveClass('expanded'); // Adjust to your UI

  // Select All
  fireEvent.click(getByText('Select All'));
  // Add assertion for all items selected, e.g.:
  // expect(...).toHaveAttribute('aria-checked', 'true'); // Adjust to your UI

  // Select None
  fireEvent.click(getByText('Select None'));
  // Add assertion for all items deselected, e.g.:
  // expect(...).toHaveAttribute('aria-checked', 'false'); // Adjust to your UI

  // Synchronize Now
  fireEvent.click(getByText('Synchronize Now'));
  // Add assertion for sync action, e.g.:
  // expect(mockSyncAction).toHaveBeenCalled(); // If you mock the sync action

  assertNockRequest(scope);
});

```

- You may need to mock or spy on Redux actions, API calls, or UI state changes to verify the expected behavior for each button.
- Adjust the assertions inside the new test to match your actual UI and state management (e.g., check for expanded/collapsed classes, selected/deselected attributes, or dispatched actions).
- If your toolbar actions trigger asynchronous behavior, consider using `await patientlyWaitFor` after each click to wait for UI updates.
</issue_to_address>

### Comment 12
<location> `test/controllers/api/v2/sync_status_controller_test.rb:31-38` </location>
<code_context>
+      ForemanTasks.stubs(:async_task).returns(build_task_stub)
+    end
+
+    def test_index
+      @controller.expects(:collect_repos).returns([])
+      @controller.expects(:collect_all_repo_statuses).returns({})
+
+      get :index, params: { :organization_id => @organization.id }
+
+      assert_response :success
+    end
+
</code_context>

<issue_to_address>
**suggestion (testing):** Missing test for unauthorized access to sync status endpoints.

Add tests to ensure unauthorized users are denied access to sync status endpoints and receive correct error responses.

```suggestion
    def test_index
      @controller.expects(:collect_repos).returns([])
      @controller.expects(:collect_all_repo_statuses).returns({})

      get :index, params: { :organization_id => @organization.id }

      assert_response :success
    end

    def test_index_unauthorized
      sign_out :user
      get :index, params: { :organization_id => @organization.id }
      assert_response :unauthorized
    end

    def test_index_forbidden
      login_user(users(:no_permissions))
      get :index, params: { :organization_id => @organization.id }
      assert_response :forbidden
    end
```
</issue_to_address>

### Comment 13
<location> `test/controllers/api/v2/sync_status_controller_test.rb:48-37` </location>
<code_context>
+      assert_response :success
+    end
+
+    def test_sync
+      @controller.expects(:latest_task).returns(nil)
+      @controller.expects(:format_sync_progress).returns({})
+
+      post :sync, params: { :repository_ids => [@repository.id], :organization_id => @organization.id }
+
+      assert_response :success
+    end
+
</code_context>

<issue_to_address>
**suggestion (testing):** Missing test for attempting to sync a non-syncable repository.

Add a test to ensure syncing a non-syncable repository returns the correct error and does not initiate a sync task.

Suggested implementation:

```ruby
    def test_sync
      @controller.expects(:latest_task).returns(nil)
      @controller.expects(:format_sync_progress).returns({})

      post :sync, params: { :repository_ids => [@repository.id], :organization_id => @organization.id }

      assert_response :success
    end

    def test_sync_non_syncable_repository
      # Create or use a repository that is not syncable
      non_syncable_repo = katello_repositories(:non_syncable_repo)
      # Ensure the repository is marked as non-syncable
      non_syncable_repo.stubs(:syncable?).returns(false)

      # Expect that no sync task is initiated
      @controller.expects(:latest_task).never
      @controller.expects(:format_sync_progress).never

      post :sync, params: { :repository_ids => [non_syncable_repo.id], :organization_id => @organization.id }

      assert_response :unprocessable_entity
      assert_match /cannot be synced/i, response.body
    end

```

- Ensure that a fixture or factory for `:non_syncable_repo` exists in your test suite, and that it is set up so that `syncable?` returns false.
- Adjust the error message assertion (`assert_match /cannot be synced/i, response.body`) to match the actual error message returned by your controller if it differs.
- If your controller returns a different error code (e.g., 400), update `assert_response :unprocessable_entity` accordingly.
</issue_to_address>

### Comment 14
<location> `webpack/scenes/SyncStatus/SyncStatusActions.js:12-13` </location>
<code_context>
  const message = getResponseErrorMsgs(error.response);
  return message;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/inline-immediately-returned-variable))

```suggestion
  return getResponseErrorMsgs(error.response);

```

<br/><details><summary>Explanation</summary>Something that we often see in people's code is assigning to a result variable
and then immediately returning it.

Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.

Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
</details>
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[jeremylenz]

@MariSvirik Updated; ping me when you have time and I can show you the changes

[ianballou]

Here's an issue with the sync bar. It happens when I'm zoomed out - when I zoom in some, the bar is within the row. Tested on Firefox.

[ianballou]

Functionally this is working great besides the loading bar issue.