Skip to content

fix: fixing route allow change admin#295

Open
miguel502 wants to merge 2 commits into
stagingfrom
bug-fix/fixing-route-allow-change-admin
Open

fix: fixing route allow change admin#295
miguel502 wants to merge 2 commits into
stagingfrom
bug-fix/fixing-route-allow-change-admin

Conversation

@miguel502
Copy link
Copy Markdown

@miguel502 miguel502 commented May 27, 2026

  1. Remove "Built By" section from landing page
  2. Make supplier table provider filter dynamic using exact matching
  3. Sort providers alphabetically on providers page, remove auto-expand behavior
  4. Restrict admin routes to authorized users only
  5. new filter on suppliers table to filter nodes by service ID
  6. delegator's own address shown in bold in services detail on services tab
  7. "All Nodes" now shows correctly after clearing status filter

Issues solved
#279
#281
#282
#285

miguel502 added 2 commits May 24, 2026 16:37
2. Make supplier table provider filter dynamic using exact matching
3. Sort providers alphabetically on providers page, remove auto-expand behavior
4. Restrict admin routes to authorized users only
2. delegator's own address shown in bold in services detail on services tab
3. "All Nodes" now shows correctly after clearing status filter
@miguel502 miguel502 changed the title Bug fix/fixing route allow change admin fix/fixing route allow change admin May 27, 2026
@miguel502 miguel502 changed the title fix/fixing route allow change admin fix: fixing route allow change admin May 27, 2026
@miguel502 miguel502 marked this pull request as draft May 27, 2026 15:48
@miguel502 miguel502 marked this pull request as ready for review May 27, 2026 15:48
@oten91 oten91 requested a review from jorgecuesta May 27, 2026 15:58
@jorgecuesta jorgecuesta added bug Something isn't working enhancement New feature or request good first issue Good for newcomers labels May 27, 2026
@jorgecuesta
Copy link
Copy Markdown
Contributor

jorgecuesta commented May 27, 2026

Code review

Found 1 issue:

  1. The change to defaultFilters makes find((f) => f.value === '') win over isDefault. But this variable is only used as the dropdown's defaultLabel (L305) — the actual filter state initialization at L109-L115 still keeps f.isDefault && f.value !== ''. Result: for the status group, the table is initialized filtered to "Staked", but the filter dropdown button will now display "All Nodes" instead of "Staked" — a UI/state mismatch. Either revert this line or also update the initial columnFilters state to match.

const currentPage = tableState.pagination.pageIndex;
const defaultFilters = filters.flatMap((filterGroup) =>
filterGroup.items.flatMap((filter) => filter.find((f) => f.value === '') || filter.find((f) => f.isDefault) || [])
);

const [columnFilters, setColumnFilters] = React.useState<ColumnFiltersState>(
filters.flatMap((filterGroup) =>
filterGroup.items.flat()
.filter((f) => f.isDefault && f.value !== '')
.map((f) => ({ id: String(f.column), value: f.value }))
)
);

@miguel502
Copy link
Copy Markdown
Author

miguel502 commented May 27, 2026

Code review

Found 1 issue:

1. The change to `defaultFilters` makes `find((f) => f.value === '')` win over `isDefault`. But this variable is only used as the dropdown's `defaultLabel` (L305) — the actual filter state initialization at L109-L115 still keeps `f.isDefault && f.value !== ''`. Result: for the status group, the table is initialized filtered to "Staked", but the filter dropdown button will now display "All Nodes" instead of "Staked" — a UI/state mismatch. Either revert this line or also update the initial `columnFilters` state to match.

const currentPage = tableState.pagination.pageIndex;
const defaultFilters = filters.flatMap((filterGroup) =>
filterGroup.items.flatMap((filter) => filter.find((f) => f.value === '') || filter.find((f) => f.isDefault) || [])
);

const [columnFilters, setColumnFilters] = React.useState<ColumnFiltersState>(
filters.flatMap((filterGroup) =>
filterGroup.items.flat()
.filter((f) => f.isDefault && f.value !== '')
.map((f) => ({ id: String(f.column), value: f.value }))
)
);

Hello @jorgecuesta, thanks for your review! I think both options (updating the initial columnFilters or reverting) would cause a problem here. The button label comes from the active filter (FilterDropdown.tsx
lines 50-53), not defaultLabel. On load columnFilters is staked, so it shows "Staked". defaultLabel ("All Nodes") only shows when the filter is cleared, which is the fix number 7 that I added on the description. So there's no init mismatch, and reverting would reintroduce the number 7 bug I added to the description problem

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

Labels

bug Something isn't working enhancement New feature or request good first issue Good for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants