Skip to content

fix(did): allow sorting dataset replicas by replication progress - #825

Open
Vrohs wants to merge 2 commits into
rucio:mainfrom
Vrohs:fix-665-sort-replication-progress
Open

fix(did): allow sorting dataset replicas by replication progress#825
Vrohs wants to merge 2 commits into
rucio:mainfrom
Vrohs:fix-665-sort-replication-progress

Conversation

@Vrohs

@Vrohs Vrohs commented Sep 1, 2026

Copy link
Copy Markdown

By submitting this PR, I confirm I have followed the Contributing Guide.

Description

Replication Progress on the dataset replicas table could not be sorted. It had sortable: false, and no field and no valueGetter, so removing the flag on its own would have sorted on nothing.

ProgressBarCell computed the percentage inline. This pulls that out into getReplicationPercentage and uses it both to render the bar and as the column's sort value, so the two cannot drift apart. An empty dataset reports 0% rather than dividing by zero.

Two side effects of giving the column a value, handled here:

  • Its filter, which defaultColDef renders on every column, was previously a no-op. Left alone it would have become a text filter matching a raw float, so it is set to agNumberColumnFilter.
  • With no field the column was addressed by position (col-id="0"). Added an explicit colId, matching colId: 'actions' / colId: 'state' elsewhere, so sort state can reference it stably.

On the rest of the issue: RegularTable sets sortable: true in its defaultColDef and StreamedTable renders RegularTable, so every table already inherits sorting. I checked the rendered headers in Storybook for the RSE, rule, subscription and DID list views and the approve-rules table, and every data column is sortable. Of the explicit sortable: false opt-outs, six look correct and are left alone: two checkbox-selection columns, two Actions button columns, and the FTS Monitoring / DDM Dashboard link columns, which have no value to sort on.

Checklist

  • This PR closes Enable sorting on all columns for tabular view #665
  • Tests cover the change, or no tests are needed
  • Documentation is updated (link the docs PR here), or no documentation change is needed
  • Database migrations are included, or the change touches no database schema
  • This PR contains no breaking changes, or the breaking change is described
    in the description and the commit follows conventional commits

Notes for contributors

  • Commit trailers: Please also link the issue in the commit message (see
    the Contributing Guide): use Closes: #____ on the commit that resolves
    the issue, and Issue: #____ on intermediate commits or if the issue
    should remain open.
  • Reviewer: After submitting, assign a reviewer if you know who is
    appropriate for the touched components; otherwise leave it empty and one
    will be assigned.
  • Stale PRs: PRs with failing tests or an unresponsive author will be
    closed promptly.

Additional notes for reviewer

Note: This OPTIONAL section is only relevant for the REVIEWER, please leave it in the PR

Reviewer template Reviewers should copy&paste the code-block below and fill it out for APPROVED pull requests. If the PR does not meet the standards the project sets out, the reasons should be WELL EXPLAINED in a CHANGE REQUEST (The answers below do not need to be answered in that case)
  • Confidence in review: I am confident in my review concerning the components this PR touches: [High 🟢, Medium 🟡 Low 🔴]
  • Confidence in scope: I am confident that this fits into the scope of the project and should be included: [High 🟢, Medium 🟡, Low 🔴]
    • For Medium and Low, explain in notes why this should be included
  • Quality: The approach is sound, maintainable and addresses the issue in the best way: [Agree 🟢]
  • Security: This PR does NOT require increased attention in terms of security (E.g. new dependencies): [Agree 🟢, Disagree 🔴]
    • If Disagree explain in notes.
  • Backwards compatibility: This PR does NOT introduce backwards compatibility breaking changes: [Agree 🟢, Disagree 🔴]
    • If Disagree explain in notes
  • Testing: This PR is well tested: [Agree 🟢]
  • Documentation: Relevant documentation or comments are updated or not required: [Agree 🟢]
- **Confidence in review**: High 🟢 Medium 🟡 Low 🔴
- **Confidence in scope**: High 🟢 Medium 🟡 Low 🔴
- **Quality**: Agree 🟢
- **Security**: Agree 🟢 Disagree 🔴
- **Backwards compatibility**: Agree 🟢 Disagree 🔴
- **Testing**: Agree 🟢
- **Documentation**: Agree 🟢

# Notes for merger



Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable sorting on all columns for tabular view

1 participant