Implementation/COMMS-1048: Labels field on the work package view with inline creation by akabiru · Pull Request #25554 · opf/openproject

akabiru · 2026-09-23T20:57:15Z

https://community.openproject.org/wp/COMMS-1048

Work packages show their labels on the full and split view, and users with edit_work_packages edit them in a multi-select that searches labels on the server and saves with the field's own save/cancel controls. A pinned option at the bottom of the dropdown creates a label from the typed name and adds it to the selection. The fetching lives in a dedicated op-labels-autocompleter so other surfaces can reuse it.

Screenshots

Full view, Details group

Initial dropdown order: used in this workspace, then most used, then name

Create option pinned below the results

Split view

AI involvement

Directed – I specified the requirements and AI implemented most of it; I validated via testing rather than a full line-by-line review.

github-actions · 2026-09-23T21:02:48Z

Deploying openproject with ⚡ PullPreview

Field	Value
Latest commit	`456a3ea`
Job	`deploy`
Status	✅ Deploy successful
Preview URL	https://pr-25554-comms-1048-labe-ip-138-199-173-233.my.opf.run:443

View logs

github-actions · 2026-09-24T14:04:26Z

Warning

Flaky specs

rspec ./spec/features/roles/report_spec.rb[1:2]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #25554, linked for reference only):

- `rspec ./spec/features/roles/report_spec.rb[1:2]`

Treat this as a standalone task, unrelated to PR #25554. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25554 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @akabiru to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @akabiru, and request a review from @akabiru.
On every commit, set @akabiru as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

github-actions · 2026-09-28T08:13:25Z

Warning

Flaky specs

rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #25554, linked for reference only):

- `rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]`

Treat this as a standalone task, unrelated to PR #25554. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25554 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @akabiru to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @akabiru, and request a review from @akabiru.
On every commit, set @akabiru as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

HDinger

Code wise this looks fine, and I only have some smaller remarks.

Conceptually, I was a bit surprised to see no permission check. It feels weird, that labels can only be edited and deleted on an admin level but everybody is allowed to add them. I was wondering what I am supposed to do when I added a typo or how to avoid duplicate creation, etc..
So I guess the main question for this PR is whether it is intended that really everybody can add new labels? So even anynomous users?

akabiru

Thanks for the useful feedback, Henriette! Sorry for the premature tag, I realized there were pending issues after requesting for review. 😅 In any case, the preliminary feedback is appreciated.

Conceptually, I was a bit surprised to see no permission check. It feels weird, that labels can only be edited and deleted on an admin level but everybody is allowed to add them. I was wondering what I am supposed to do when I added a typo or how to avoid duplicate creation, etc..
So I guess the main question for this PR is whether it is intended that really everybody can add new labels? So even anynomous users?

The missing permission is an oversight: it's currently only enforced at the API level. Only users with edit_work_packages permissions should be able to add/create labels.

Duplicate creation is handled at the API level in #25553 via a Labels::FindOrCreateService that is concurrency safe: a case-insensitive unique index on the name, so a create that differs only in casing, or races another request, returns the existing label.

we have a rule that any user-created label that isn't in use is also removed from the admin list of labels, you could then just remove the accidentally-created label and it would be "deleted" (assuming that no one else uses "OpenPrk" between the time you accidentally created the label and when you remove it).

Atm, typos are handled by the above rule (see FND-5/activity#comment-1735504), albeit not implemented yet.

github-actions · 2026-09-29T07:37:15Z

Warning

Flaky specs

rspec ./modules/bim/spec/features/card_view/select_card_spec.rb[1:1:1]
rspec ./modules/bim/spec/features/card_view/select_card_spec.rb[1:1:3]
rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #25554, linked for reference only):

- `rspec ./modules/bim/spec/features/card_view/select_card_spec.rb[1:1:1]`
- `rspec ./modules/bim/spec/features/card_view/select_card_spec.rb[1:1:3]`
- `rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]`

Treat this as a standalone task, unrelated to PR #25554. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25554 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @akabiru to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @akabiru, and request a review from @akabiru.
On every commit, set @akabiru as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

akabiru

Follow-up on the conceptual questions:

Permissions (cbfd2ae, f0f777b):

Users with view_work_packages only see labels read-only.
With add_work_packages but not edit_work_packages, users can pick existing labels when creating a work package. They can't edit labels on existing work packages or create new labels.
Creating new labels on the fly is offered only to users with edit_work_packages in the project. The API enforces the same rule, so anonymous users can only create labels if an admin grants that permission to the anonymous role. The client no longer special-cases permission errors (4c43553).

Duplicates: handled at the API level in #25553 by Labels::FindOrCreateService. It relies on a case-insensitive unique index on the name, and a create that only differs in casing, or races another request, returns the existing label.

Typos: the rule from FND-5 (unused user-created labels drop out of the admin list) is tracked in COMMS-1050. It needs label journaling (#25320) first, so labels that were used before are never deleted.

akabiru · 2026-09-29T10:00:05Z

🚧 Failing tests fixed in #25655

Labels render like the other multi-value attributes and are edited in a multi-select that searches on the server so the workspace-relevance ordering is kept. A dedicated labels autocompleter holds the fetching so other surfaces can reuse it.

The create option is pinned below the options and only offered while the typed name matches no loaded label. The created label joins the open selection and is saved with the field.

The accessible name of the field stringified the HAL resources; it now joins the names like the title does. The create-label option also stays hidden while results are still loading.

The create action is an addTag option, so arrow keys and Enter reach it. A 403 on create shows a specific message instead of the raw error.

Save/Cancel render only alongside the input, and the combobox is announced by the field name.

The API authorizes label creation, so the client no longer special-cases 403 responses.

New labels read "<name> (New label)", matches highlight in blue, and the list closes on pick so it can open either way without covering Save/Cancel.

The create option stays fixed while the list scrolls and remains a keyboard-reachable option.

The shared schema no longer treats add_work_packages as enough to edit labels, so add-only users see the field read-only.

Add-only users pick existing labels; creating new ones on the fly needs edit_work_packages in the project.

akabiru · 2026-09-29T10:30:24Z

+// ng-select gives the addTag row no class of its own; :has() reaches it via
+// the marker element in the labels tag template.
+.op-labels-autocompleter--panel
+  .ng-option:has(> .labels-autocompleter--create-option)
+    position: sticky
+    bottom: 0
+    border-top: 1px solid var(--borderColor-default)
+
+    // The hover token is translucent; layering it over the body background keeps the pinned row opaque.
+    &.ng-option-marked
+      background-color: var(--body-background) !important
+      background-image: linear-gradient(var(--control-transparent-bgColor-hover), var(--control-transparent-bgColor-hover)) !important
+


A note on the custom styles for the "(New label)" row, since the assignee field pins its "Invite user" button without any.

The invite button lives in ng-select's footer slot, which renders outside the scrolling list, so it stays put for free. The create option here is ng-select's addTag option instead, so it renders inside the list as the last option. That keeps it a real option: arrow keys and Enter reach it, and when nothing matches it's the marked row, so Enter creates the label (the behaviour described in FND-5). A footer button can't be reached by keyboard at all.

Pinning something that lives inside the scroll container needs a bit of help:

position: sticky; bottom: 0 keeps the row at the bottom edge of the list while labels scroll underneath (_autocomplete.sass). ng-select gives the row no class of its own, so :has() finds it through a marker span in our tag template. The rule is scoped to the labels panel via classes, since the panel is appended to body.

The marked state layers the translucent hover token over the body background, otherwise scrolled labels show through the pinned row.

virtualScroll is off for this autocompleter, because virtual scrolling moves the list with transforms and breaks sticky. Labels are fetched in full already, so this only changes rendering.

keydowned() nudges the list when arrowing, because ng-select scrolls the marked option flush with the bottom edge, where the pinned row now sits.

All of it is limited to the labels autocompleter; other autocompleters are unaffected.

Thanks for the detailed explanation. 🙇

I have to admit, that I am really not a big fan of that for several reasons:

We already have a style for these kind of adding new elements. Look for example in the share WP dialog. Why can't we re-use the same style instead of adding again another one? The text is blue and in front of the option (instead of grey and behind it). Further, the background remains unchanged (instead of greyed).

A grey background ususally means disabled, which is not what you want to communicate. That somehow belongs to point 1. Further, why did you use background-image instead of just setting a background-color?

I don't really get the necessity for making the create option sticky. Imho, this makes this way to prominent, as if this is the desired thing to do instead of picking an exisiting label. But it is actually the other way around, isn't it? The stickyness also makes it feel weird to navigate with the arrow keys because it feels like I can actual never reach the element due to the separator but suddenly I can..

You are using a hover color for a non-hover state

Name the permission each schema case covers and check the case-insensitive duplicate on its own.

The joined name backs the title and aria-label of every multi-value resource field, not only labels.

github-actions · 2026-09-29T11:08:04Z

Warning

Flaky specs

rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #25554, linked for reference only):

- `rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]`

Treat this as a standalone task, unrelated to PR #25554. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25554 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @akabiru to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @akabiru, and request a review from @akabiru.
On every commit, set @akabiru as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

HDinger

Looks good overall. My biggest concern is regarding the special styling of the create option. I'd like to see that harmonized with other places we have, please see my comment below.

One other thing I found: When a label is very long, it pushes the rest out of the field. In the example below, I have two labels selected, but I only see one:

This is probably an exisiting problem which also affects other fields, so I am fine with keeping that out of scope for this PR.

HDinger · 2026-09-30T08:48:55Z

+// ng-select gives the addTag row no class of its own; :has() reaches it via
+// the marker element in the labels tag template.
+.op-labels-autocompleter--panel
+  .ng-option:has(> .labels-autocompleter--create-option)
+    position: sticky
+    bottom: 0
+    border-top: 1px solid var(--borderColor-default)
+
+    // The hover token is translucent; layering it over the body background keeps the pinned row opaque.
+    &.ng-option-marked
+      background-color: var(--body-background) !important
+      background-image: linear-gradient(var(--control-transparent-bgColor-hover), var(--control-transparent-bgColor-hover)) !important
+


Thanks for the detailed explanation. 🙇

I have to admit, that I am really not a big fan of that for several reasons:

We already have a style for these kind of adding new elements. Look for example in the share WP dialog. Why can't we re-use the same style instead of adding again another one? The text is blue and in front of the option (instead of grey and behind it). Further, the background remains unchanged (instead of greyed).

A grey background ususally means disabled, which is not what you want to communicate. That somehow belongs to point 1. Further, why did you use background-image instead of just setting a background-color?

I don't really get the necessity for making the create option sticky. Imho, this makes this way to prominent, as if this is the desired thing to do instead of picking an exisiting label. But it is actually the other way around, isn't it? The stickyness also makes it feel weird to navigate with the arrow keys because it feels like I can actual never reach the element due to the separator but suddenly I can..

You are using a hover color for a non-hover state

akabiru added this pull request to stack #25555 September 23, 2026 20:57

akabiru changed the title ~~implementation/comms 1048 labels field on work package view~~ Implementation/COMMS-1048: Labels field on the work package view with inline creation Sep 23, 2026

akabiru self-assigned this Sep 23, 2026

akabiru mentioned this pull request Sep 23, 2026

Feature/COMMS-1004: Labels for work package show page #25551

Closed

akabiru added the ai: Collaborative 💻 AI generated a substantial part of the code; A human reviewed and understands every line. label Sep 23, 2026

akabiru added this to the 18.0.x milestone Sep 23, 2026

github-actions Bot removed the ai: Collaborative 💻 AI generated a substantial part of the code; A human reviewed and understands every line. label Sep 23, 2026

akabiru added the pullpreview label Sep 23, 2026

akabiru added ai: Collaborative 💻 AI generated a substantial part of the code; A human reviewed and understands every line. and removed pullpreview labels Sep 23, 2026

akabiru force-pushed the implementation/comms-1048-labels-field-on-work-package-view branch from b47e919 to a6d038f Compare September 24, 2026 06:13

github-actions Bot removed the ai: Collaborative 💻 AI generated a substantial part of the code; A human reviewed and understands every line. label Sep 24, 2026

akabiru force-pushed the implementation/comms-1048-labels-field-on-work-package-view branch 2 times, most recently from 483d2b9 to 04b74a4 Compare September 24, 2026 13:39

akabiru force-pushed the implementation/comms-1048-labels-field-on-work-package-view branch from 04b74a4 to 60c6bcd Compare September 24, 2026 14:28

akabiru added ai: Directed 🪄 A human specified the requirements and AI implemented most of it; They validated via testing. pullpreview labels Sep 28, 2026

akabiru marked this pull request as ready for review September 28, 2026 08:11

github-actions Bot removed the ai: Directed 🪄 A human specified the requirements and AI implemented most of it; They validated via testing. label Sep 28, 2026

akabiru requested review from a team and HDinger September 28, 2026 08:11

akabiru added the ai: Directed 🪄 A human specified the requirements and AI implemented most of it; They validated via testing. label Sep 28, 2026

akabiru marked this pull request as draft September 28, 2026 08:49

akabiru removed request for a team and HDinger September 28, 2026 08:49

HDinger requested changes Sep 28, 2026

View reviewed changes

akabiru commented Sep 28, 2026

View reviewed changes

github-actions Bot added ai: Directed 🪄 A human specified the requirements and AI implemented most of it; They validated via testing. and removed ai: Directed 🪄 A human specified the requirements and AI implemented most of it; They validated via testing. labels Sep 29, 2026

akabiru commented Sep 29, 2026

View reviewed changes

akabiru marked this pull request as ready for review September 29, 2026 10:00

akabiru added 10 commits September 29, 2026 13:01

Create labels from the work package labels dropdown

f492f4d

The create option is pinned below the options and only offered while the typed name matches no loaded label. The created label joins the open selection and is saved with the field.

Announce multi-value display fields by name

6046f8f

The accessible name of the field stringified the HAL resources; it now joins the names like the title does. The create-label option also stays hidden while results are still loading.

Create labels from a keyboard-reachable dropdown option

8fd154f

The create action is an addTag option, so arrow keys and Enter reach it. A 403 on create shows a specific message instead of the raw error.

Hide label edit controls without an autocompleter URL

40000f5

Save/Cancel render only alongside the input, and the combobox is announced by the field name.

Leave label create errors to the generic handler

5af7db1

The API authorizes label creation, so the client no longer special-cases 403 responses.

Match the labels dropdown to the assignee autocompleter

7c4b670

New labels read "<name> (New label)", matches highlight in blue, and the list closes on pick so it can open either way without covering Save/Cancel.

Pin the create label option to the bottom of the dropdown

049e09c

The create option stays fixed while the list scrolls and remains a keyboard-reachable option.

Require edit rights for labels on existing work packages

6fe4394

The shared schema no longer treats add_work_packages as enough to edit labels, so add-only users see the field read-only.

Offer label creation only to users who can edit work packages

0936db9

Add-only users pick existing labels; creating new ones on the fly needs edit_work_packages in the project.

akabiru force-pushed the implementation/comms-1048-labels-field-on-work-package-view branch from f0f777b to 0936db9 Compare September 29, 2026 10:01

akabiru commented Sep 29, 2026

View reviewed changes

akabiru added 2 commits September 29, 2026 13:43

Tidy the labels specs

3a31f2e

Name the permission each schema case covers and check the case-insensitive duplicate on its own.

Cover version fields in the multi-value display name

456a3ea

The joined name backs the title and aria-label of every multi-value resource field, not only labels.

akabiru requested review from a team and HDinger September 29, 2026 10:46

HDinger reviewed Sep 30, 2026

View reviewed changes

Conversation

akabiru commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Screenshots

AI involvement

Uh oh!

github-actions Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying openproject with ⚡ PullPreview

Uh oh!

github-actions Bot commented Sep 24, 2026

Uh oh!

github-actions Bot commented Sep 28, 2026

Uh oh!

HDinger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akabiru left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 29, 2026

Uh oh!

akabiru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akabiru commented Sep 29, 2026

Uh oh!

akabiru Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

HDinger Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 29, 2026

Uh oh!

HDinger left a comment

Choose a reason for hiding this comment

Uh oh!

HDinger Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

akabiru commented Sep 23, 2026 •

edited

Loading

github-actions Bot commented Sep 23, 2026 •

edited

Loading

akabiru left a comment •

edited

Loading