Feature/COMMS-1025: Labels administration - #25394
Merged
Merged
Conversation
akabiru
added this pull request to stack #25396
September 16, 2026 19:09
akabiru
removed this pull request from stack #25396
September 17, 2026 05:49
akabiru
force-pushed
the
implementation/comms-1025-label-admin-listing
branch
from
September 17, 2026 05:52
3345955 to
ed03c50
Compare
akabiru
changed the base branch from
implementation/comms-1025-labels-feature-flag
to
implementation/comms-1025-labels-services-and-contracts
September 17, 2026 05:52
akabiru
added this pull request to stack #25402
September 17, 2026 05:52
akabiru
force-pushed
the
implementation/comms-1025-label-admin-listing
branch
from
September 17, 2026 06:21
ed03c50 to
204c1d4
Compare
akabiru
force-pushed
the
implementation/comms-1025-label-admin-listing
branch
from
September 17, 2026 07:28
204c1d4 to
07d5c3b
Compare
Base automatically changed from
implementation/comms-1025-labels-services-and-contracts
to
dev
September 17, 2026 12:26
Admins get a paginated, searchable list of all labels with their usage and can create, rename and delete labels from dialogs. The page sits behind the work_package_labels flag and is admin-only.
The list is a BorderBox table as the FND-5 epic specifies, the delete dialog names how many work packages are affected, name errors read as full sentences, and creating a label lands on the page that contains it.
Walks through navigation, create, rename, delete, search with pagination, and the flag-off case in the browser.
Long label names truncate instead of overflowing the row, the page lookup after creating a label compares case-insensitively in SQL only, and the dialog, form and delete-failure paths gain specs.
Cuprite in CI did not observe the stream-rendered event within the wait window; waiting for network idle after typing is what the other admin search specs do.
akabiru
force-pushed
the
implementation/comms-1025-label-admin-listing
branch
from
September 17, 2026 12:26
dba38fe to
80ce9f7
Compare
akabiru
marked this pull request as ready for review
September 17, 2026 13:10
Component specs now use the test selector helpers, the accessible role selectors and a new :async_dialog_trigger selector instead of raw DOM queries, and stop auditing class lists and Stimulus wiring.
Renaming can move a label across an alphabetical page boundary, and landing back on page one hid the label the admin just renamed. Update now shares the page computation create already used.
The create and rename dialogs carried a state flag that only ever mirrored whether the label was persisted, and the form component accepted it unvalidated. Both now read the record instead. The form component spec builds an unsaved label for the failed-create render, since a stubbed one reports itself persisted and rendered the rename form.
Checking the index body for the word "Labels" passed on the page title alone. Both flag states now inspect the sidebar anchors.
The name field is required, so the browser blocked the submit before any request and the assertions passed without exercising the server. The request spec covers the blank-name rejection.
Cuprite fires no input event when filling a field with an empty string, so the live search never reloaded and the example failed deterministically. The clear button dispatches the input event the filter form listens for.
Authorization, flag gating on mutations, the search pagination link target, page redirects after create and rename, and the delete failure branch stay; user flows live in the feature spec.
Deploying openproject with ⚡ PullPreview
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Address the unsafe HTML interpolation and preserve the selected page size in the index URL.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds feature-flagged administration for work package labels, including listing, searching, pagination, creation, renaming, and deletion.
Changes:
- Added routes, navigation, feature gating, controllers, forms, and translations.
- Added label administration components and dialogs.
- Added request, feature, model, and component coverage.
Review findings:
- Critical: Escape
label.namebefore interpolating it into the HTML-safe deletion confirmation. - Moderate: Preserve
per_pagein the pushed index URL.
File summaries
| File | Description |
|---|---|
spec/support/finders/test_selector_finders.rb |
Request-spec selector support |
spec/support/capybara/async_dialog_selector.rb |
Async dialog selectors |
spec/requests/admin/labels_spec.rb |
Request behavior coverage |
spec/models/label_spec.rb |
Pagination coverage |
spec/features/admin/labels/manage_spec.rb |
Label management workflows |
spec/components/admin/labels/table_component_spec.rb |
Table rendering coverage |
spec/components/admin/labels/sub_header_component_spec.rb |
Search and create controls |
spec/components/admin/labels/row_component_spec.rb |
Row actions coverage |
spec/components/admin/labels/list_component_spec.rb |
List and empty states |
spec/components/admin/labels/form_component_spec.rb |
Form coverage |
spec/components/admin/labels/dialog_component_spec.rb |
Create and rename dialogs |
spec/components/admin/labels/delete_dialog_component_spec.rb |
Delete confirmation coverage |
config/routes.rb |
Label administration routes |
config/locales/en.yml |
Label administration translations |
config/initializers/menus.rb |
Feature-flagged admin menu |
app/views/admin/labels/index.html.erb |
Labels administration page |
app/models/label.rb |
Usage counts and pagination |
app/forms/admin/labels/form.rb |
Label form definition |
app/controllers/concerns/labels/labels_feature.rb |
Feature flag enforcement |
app/controllers/admin/labels_controller.rb |
Administration actions |
app/components/admin/labels/table_component.rb |
Labels table |
app/components/admin/labels/sub_header_component.rb |
Subheader behavior |
app/components/admin/labels/sub_header_component.html.erb |
Subheader rendering |
app/components/admin/labels/row_component.rb |
Label row rendering |
app/components/admin/labels/list_component.rb |
List behavior |
app/components/admin/labels/list_component.html.erb |
List and empty states |
app/components/admin/labels/form_component.rb |
Form behavior |
app/components/admin/labels/form_component.html.erb |
Form rendering |
app/components/admin/labels/dialog_component.rb |
Dialog behavior |
app/components/admin/labels/dialog_component.html.erb |
Create and rename dialogs |
app/components/admin/labels/delete_dialog_component.rb |
Delete dialog behavior |
app/components/admin/labels/delete_dialog_component.html.erb |
Delete confirmation rendering |
Review details
Suppressed comments (1)
app/controllers/admin/labels_controller.rb:54
- The pushed index URL drops
per_page, even though the search request uses it to render the current page size. After searching, reloading or navigating back to the pushed URL silently resets the user's selected page size to the default. Preserveper_pagein the permitted parameters, consistent with the existing index controllers (app/controllers/users_controller.rb:481andapp/controllers/placeholder_users_controller.rb:199).
turbo_streams << turbo_stream.push_state(url_for(params.permit(:controller, :filters).merge(action: "index")))
- Files reviewed: 32/32 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
brunopagno
approved these changes
Sep 18, 2026
brunopagno
left a comment
brunopagno
left a comment
Contributor
There was a problem hiding this comment.
All good from my side. Played around a bit on pull preview and it's looking quite nice 🎉
Only remark, I find the usage of Arel to obfuscate a bit the code. I'd prefer to avoid it, but I think it's okay in the context it's being used.
The empty and no-matches states stay inside the table so the column headers remain visible.
Drops the single-use Arel scope; the redirect target reads as one comparison against the case-insensitive listing order.
akabiru
added
the
ai: Collaborative 💻
github-actions
Bot
removed
the
ai: Collaborative 💻
Member
Author
|
🚧 Failing test is a known issue that is fixed in #25519 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://community.openproject.org/wp/COMMS-1025
https://community.openproject.org/wp/COMMS-1026
Adds the labels administration page behind the
work_package_labelsflag: dialogs to create, rename and delete labels.