EPMRPP-103879 || Add sorting for organizations on 'All organizations' page #4409
Conversation
WalkthroughThis update introduces a comprehensive sorting feature for organizations, including new UI components, permission controls, analytics events, and localization entries. TypeScript types, constants, and SCSS modules are added to support the dropdown sorting interface and ensure accessibility and internationalization. Related components and tables are refactored to integrate sorting state and logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OrganizationsPageHeader
participant OrganizationsSorting
participant DropdownSorting
participant OrganizationsPanelView
participant OrganizationsTable
User->>OrganizationsPageHeader: Loads organizations page
OrganizationsPageHeader->>OrganizationsSorting: (if permitted) Render sorting UI
OrganizationsSorting->>DropdownSorting: Render dropdown with sorting options
User->>DropdownSorting: Selects sorting option/direction
DropdownSorting->>OrganizationsSorting: onChange({value, direction})
OrganizationsSorting->>OrganizationsPanelView: onChangeSorting(field)
OrganizationsPanelView->>OrganizationsTable: Pass sortingColumn, sortingDirection
OrganizationsTable->>OrganizationsTable: Sort data and render table
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (30)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (25)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4409 +/- ##
===========================================
- Coverage 66.56% 66.53% -0.04%
===========================================
Files 85 85
Lines 981 983 +2
Branches 140 140
===========================================
+ Hits 653 654 +1
- Misses 297 298 +1
Partials 31 31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
allaprischepa
requested review from
AmsterGet,
BlazarQSO and
maria-hambardzumian
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (16)
app/src/common/constants/keyCodes.js (1)
38-50: Consider freezing the keys object and aligning naming.Rename
Keysto uppercaseKEYSand wrap it withObject.freezeto prevent runtime mutation, aligning with other constants:- export const Keys = { + export const KEYS = Object.freeze({ BACKSPACE: 'Backspace', TAB: 'Tab', ENTER: 'Enter', ESCAPE: 'Escape', SPACE: ' ', ARROW_LEFT: 'ArrowLeft', ARROW_UP: 'ArrowUp', ARROW_RIGHT: 'ArrowRight', ARROW_DOWN: 'ArrowDown', DELETE: 'Delete', - }; + } as const);app/src/controllers/sorting/types.ts (2)
25-26: RefinenamespaceandnamespaceSelectorinstead ofunknown.Using
unknownis overly permissive. Consider specifying concrete types or making the interface generic to reflect actual usage (e.g.string, selector function, or a typed key).
21-28: Add JSDoc for sorting props.Documenting each property in
WithSortingURLPropswill improve readability and help future maintainers understand how these URL props are used.app/src/common/utils/permissions/permissions.js (1)
89-91: Add unit tests for the new permission check.
canWorkWithOrganizationsSortingisn’t covered by existing tests. Please add cases inpermissions.test.jsto validate its behavior for administrator, manager, and other roles.Would you like me to draft the test skeleton?
app/localization/translated/uk.json (1)
1646-1650: Verify terminology consistency for "users".Here you use “Учасники” for the
userslabel; elsewhere in the UI the term “Користувачі” may be present. Align on one term across all Ukrainian translations.app/src/components/main/analytics/events/ga4Events/organizationsPageEvents.js (1)
41-46: Align dynamic-event function name with existing naming convention
Existing dynamic generators (e.g.,clickApplyFilterButton) start with a verb, while the newly-addedorganizationsSortingdoes not. For consistency and easier grep/search, consider renaming to something likeclickOrganizationsSorting(orselectOrganizationsSorting).- organizationsSorting: (type) => ({ + clickOrganizationsSorting: (type) => ({app/.eslintrc (1)
108-121: Duplicate / conflicting rule declarations
import/extensionsandimport/prefer-default-exportare already defined in the root ruleset (lines 37 and 68-77). Duplicating them inside the TypeScript override adds noise and can mislead future maintainers when the two blocks drift out of sync. Consider removing the duplicate entries from the override.app/src/components/dropdownSorting/dropdownSorting.scss (1)
55-57: Removing focus outline harms keyboard accessibility
.select-list:focus { outline: none; }eliminates the only visible focus indication for users navigating with a keyboard. Replace it with a custom outline style instead of removing it entirely.-.select-list:focus { - outline: none; -} +.select-list:focus { + outline: 2px solid $COLOR--topaz; + outline-offset: 2px; +}app/src/pages/instance/organizationsPage/organizationsPageHeader/organizationsPageHeader.jsx (1)
32-33: Import path inconsistency
Other permission helpers are imported from'common/utils/permissions', while the new one uses'common/utils/permissions/permissions'. Mixing both styles will trip relative-path regex searches and may cause circular-import headaches later. Prefer the shorter, already-used path:-import { canWorkWithOrganizationsSorting } from 'common/utils/permissions/permissions'; +import { canWorkWithOrganizationsSorting } from 'common/utils/permissions';Also applies to: 38-38
app/src/components/dropdownSorting/dropdownSortingOption/dropdownSortingOption.scss (1)
57-61: Icon rotation collides with explicit down-arrowWhen a row is both
.selectedand hovered you rotate the icon 180°.
Ifdirection === SORTING_DESCthe component is already rendering a real down-arrow (see TSX). Rotating the up-arrow on hover produces inconsistent visual feedback.Consider dropping the rotation or limiting it to the non-selected state.
app/src/components/dropdownSorting/dropdownSortingOption/dropdownSortingOption.tsx (1)
61-75: Add basic ARIA attributes for accessibilityThe option buttons are keyboard-focusable via script but lack semantics for screen readers.
- <button + <button + role="menuitemradio" + aria-checked={selected} + aria-label={label} type="button"These attributes make the dropdown navigable by assistive technologies without affecting current behaviour.
app/src/pages/instance/organizationsPage/organizationsPanelView/organizationsPanelView.jsx (1)
82-85: Tighten prop-types for the new sorting propsUsing plain
stringloses compile-time validation. Declare the allowed literals and mark the callback as required to match its usage.- sortingColumn: PropTypes.string, - sortingDirection: PropTypes.string, - onChangeSorting: PropTypes.func, + sortingColumn: PropTypes.oneOf([ + 'created_at', + 'name', + 'users', + 'projects', + 'last_launch_date', + ]), + sortingDirection: PropTypes.oneOf(['asc', 'desc']), + onChangeSorting: PropTypes.func.isRequired,app/src/components/dropdownSorting/dropdownSorting.tsx (2)
85-108: ESC key should close the list for full keyboard controlUsers expect
Escto collapse dropdowns. Adding it improves UX and aligns with ARIA practices.if ([Keys.ENTER, Keys.SPACE].includes(event.key)) { event.preventDefault(); handleChange(options[highlightedIndex].value); } + if (event.key === Keys.ESCAPE) { + event.preventDefault(); + setIsOpened(false); + }
138-148: Expose dropdown state to assistive techAdd
aria-haspopupandaria-expandedto the trigger button to announce the popover state.<button type="button" className={cx('value', { open: isOpened })} + aria-haspopup="listbox" + aria-expanded={isOpened} onKeyDown={() => setKeyboardControl(true)} >app/src/pages/instance/organizationsPage/organizationsPanelView/organizationsTable/organizationsTable.jsx (2)
48-105:useMemowill recalculate on every assigned-organizations change
assignedOrganizationsis an object that changes reference whenever any organization assignment mutates, which may be frequent. This forces the whole table data to be remapped on each such change, potentially degrading performance for large lists.
Consider memoising / normalisingassignedOrganizationsin Redux or deriving only the needed slices (e.g. a shallow array of organisation roles) before putting it into the dependency list.
155-162: Minor: early‐return could guard againstnull-ish keys
getSortingColumnis called with whatever comes from URL params. A short guard can avoid the extra lookup whenkeyisnull/undefinedand helps future maintainers. Not critical, but improves readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
app/.eslintrc(1 hunks)app/localization/translated/be.json(1 hunks)app/localization/translated/es.json(1 hunks)app/localization/translated/ru.json(1 hunks)app/localization/translated/uk.json(1 hunks)app/localization/translated/zh.json(1 hunks)app/package.json(1 hunks)app/src/common/constants/keyCodes.js(1 hunks)app/src/common/constants/permissions.js(1 hunks)app/src/common/utils/permissions/permissions.js(1 hunks)app/src/components/dropdownSorting/dropdownSorting.scss(1 hunks)app/src/components/dropdownSorting/dropdownSorting.tsx(1 hunks)app/src/components/dropdownSorting/dropdownSortingOption/dropdownSortingOption.scss(1 hunks)app/src/components/dropdownSorting/dropdownSortingOption/dropdownSortingOption.tsx(1 hunks)app/src/components/dropdownSorting/dropdownSortingOption/index.ts(1 hunks)app/src/components/dropdownSorting/index.ts(1 hunks)app/src/components/main/analytics/events/ga4Events/organizationsPageEvents.js(1 hunks)app/src/controllers/instance/organizations/constants.ts(2 hunks)app/src/controllers/sorting/types.ts(1 hunks)app/src/pages/instance/organizationsPage/organizationsPageHeader/organizationsPageHeader.jsx(3 hunks)app/src/pages/instance/organizationsPage/organizationsPageHeader/organizationsSorting/index.ts(1 hunks)app/src/pages/instance/organizationsPage/organizationsPageHeader/organizationsSorting/messages.ts(1 hunks)app/src/pages/instance/organizationsPage/organizationsPageHeader/organizationsSorting/organizationsSorting.tsx(1 hunks)app/src/pages/instance/organizationsPage/organizationsPanelView/organizationsPanelView.jsx(5 hunks)app/src/pages/instance/organizationsPage/organizationsPanelView/organizationsTable/organizationsTable.jsx(6 hunks)app/src/types/global.d.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/src/common/utils/permissions/permissions.js (2)
app/src/common/utils/permissions/permissions.test.js (1)
checkPermission(32-32)app/src/common/constants/permissions.js (2)
ACTIONS(22-70)ACTIONS(22-70)
app/src/pages/instance/organizationsPage/organizationsPageHeader/organizationsPageHeader.jsx (2)
app/src/common/utils/permissions/permissions.js (2)
canWorkWithOrganizationsSorting(89-91)canWorkWithOrganizationsSorting(89-91)app/src/pages/instance/organizationsPage/organizationsPageHeader/organizationsSorting/organizationsSorting.tsx (1)
OrganizationsSorting(76-76)
🔇 Additional comments (14)
app/src/types/global.d.ts (1)
17-20: Add global SCSS module declaration.Declaring
.scssimports as typed objects ensures type safety and editor support for CSS modules. Well structured and necessary for the new dropdown sorting styles.app/src/controllers/instance/organizations/constants.ts (2)
2-2: License header year bump.Updated the Apache license header year from 2024 to 2025. No further action needed.
37-46: IntroduceSortingFieldsenum and default sort column.Defines clear, strongly-typed sorting keys and sets a sensible default. This provides a solid foundation for the organizations sorting feature.
app/localization/translated/ru.json (1)
1647-1651: I want to ensure we didn’t missen.jsonin the scan. Let’s list all locale files and verify the new keys are present inen.jsonas well.#!/bin/bash # List all locale files under translated echo "Locale files:" ls app/localization/translated/*.json # Check that en.json contains the OrganizationsSorting keys echo -e "\nChecking OrganizationsSorting keys in en.json:" rg "\"OrganizationsSorting\\.(creationDate|lastLaunchDate|name|projects|users)\"" -n app/localization/translated/en.json || echo "❌ en.json is missing these keys"app/src/components/dropdownSorting/index.ts (1)
1-18: Re-export and license header look goodThe license header is applied correctly, and the
export { DropdownSorting } from './dropdownSorting';statement provides a clean, consistent import path for the component.app/localization/translated/uk.json (2)
1646-1650: Localization entries are correctly added and ordered.The new
OrganizationsSorting.*keys are inserted in alphabetical order with accurate Ukrainian translations and proper JSON syntax.
1646-1650: Ensure translation keys are consumed in code.Please verify that each of these new keys is actually referenced by the sorting component to avoid dead entries.
#!/bin/bash # Verify usage of new sorting translation keys in the codebase rg -n "OrganizationsSorting\.(creationDate|lastLaunchDate|name|projects|users)"app/src/pages/instance/organizationsPage/organizationsPageHeader/organizationsSorting/index.ts (1)
17-17: Clean re-export of OrganizationsSorting
This module correctly re-exports theOrganizationsSortingcomponent, ensuring concise and consistent imports across the codebase.app/src/components/dropdownSorting/dropdownSortingOption/index.ts (1)
17-17: Consistent re-export of DropdownSortingOption
The index file provides a straightforward re-export ofDropdownSortingOption, facilitating cleaner import paths.app/localization/translated/be.json (1)
1647-1651: Confirm accuracy and consistency of Belarusian translations for new sorting optionsEnsure that the translation entries under
OrganizationsSortingmatch the intended sorting labels and align with translations in other locales. The current mappings:
- "creationDate": "Дата стварэння"
- "lastLaunchDate": "Дата апошняга запуску"
- "name": "Назва"
- "projects": "Праекты"
- "users": "Удзельнікі"
Everything appears correct and consistent.
app/src/pages/instance/organizationsPage/organizationsPageHeader/organizationsSorting/messages.ts (1)
19-40: Messages file looks good
I don’t see any i18n, typing or casing issues.app/src/pages/instance/organizationsPage/organizationsPageHeader/organizationsPageHeader.jsx (1)
91-92: Conditional render looks good, but verify header spacing
Rendering<OrganizationsSorting />inside the existing flex container may shift the layout when permissions vary. Double-check the header spacing in both “allowed” and “no-permission” cases.app/src/pages/instance/organizationsPage/organizationsPanelView/organizationsTable/organizationsTable.jsx (2)
163-169: Permission helper receives onlyuserRole
canWorkWithOrganizationsSortingis fed an object containing just{ userRole }. Verify that this helper doesn’t expect additional params (e.g. projectRole) to avoid a false-negative permission check.
171-174: Handler ignores sorting direction
handleChangeSortingforwards only the column key. If the dropdown is also expected to toggle direction, the table cannot reflect it. Double-check the contract ofonChangeSorting; you might need to pass bothkeyanddirection.
app/src/components/dropdownSorting/dropdownSortingOption/dropdownSortingOption.scss
Outdated
Show resolved
Hide resolved
...ance/organizationsPage/organizationsPageHeader/organizationsSorting/organizationsSorting.tsx
Show resolved
Hide resolved
.../instance/organizationsPage/organizationsPanelView/organizationsTable/organizationsTable.jsx
Show resolved
Hide resolved
.../instance/organizationsPage/organizationsPanelView/organizationsTable/organizationsTable.jsx
Show resolved
Hide resolved
app/src/components/dropdownSorting/dropdownSortingOption/dropdownSortingOption.scss
Outdated
Show resolved
Hide resolved
app/src/components/dropdownSorting/dropdownSortingOption/dropdownSortingOption.tsx
Outdated
Show resolved
Hide resolved
app/src/components/dropdownSorting/dropdownSortingOption/dropdownSortingOption.tsx
Outdated
Show resolved
Hide resolved
app/src/components/dropdownSorting/dropdownSortingOption/dropdownSortingOption.tsx
Outdated
Show resolved
Hide resolved
allaprischepa
force-pushed
the
feature/EPMRPP-103879-all-organizations-sorting
branch
from
6a3ccff to
3524268
Compare
|
|
Force pushed after rebasing |
allaprischepa
deleted the
feature/EPMRPP-103879-all-organizations-sorting
branch
PR Checklist
developfor features/bugfixes, other if mentioned in the task)npm run lint) prior to submission? Enable the git hook on commit in your IDE to run it and format the code automatically.npm run build)?devnpm script)manage:translationsscript?Changes
DropdownSortingcomponent.eslintrc:import/prefer-default-exportruleno-shadowrule as the default one has incompatibility with TypeScriptimport/extensionsrule to allow skipping file extensions for JS/TS files while importingToDo
DropdownSortingcomponent to remove the popover center alignmentDepends on: EPMRPP-103879 || Support optional center alignment ui-kit#100
toLowerCase()conversion forsortingDirectionvalues passed to the UI-KitTablecomponentDepends on: EPMRPP-103879 || Make sort direction case insensitive and enum free ui-kit#103
Visuals
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Localization
Enhancements
Chores
@reportportal/ui-kitand added type definitions forreact-tracking.