[#73798] Remove scrum_projects feature flag from Backlogs#22740
[#73798] Remove scrum_projects feature flag from Backlogs#22740
scrum_projects feature flag from Backlogs#22740Conversation
There was a problem hiding this comment.
Pull request overview
Removes the scrum_projects feature decision from the Backlogs module and makes the sprint-based Backlogs (Agile::Sprint) behavior the default across UI routes, controllers, helpers, and API endpoints, with corresponding spec updates.
Changes:
- Removed
scrum_projectsfeature decision registration and deleted feature-flag-dependent branches across Backlogs routes/controllers/API. - Made sprint-based Backlogs behavior the default (Agile::Sprint), including routing and representer/schema exposure.
- Updated Backlogs and PDF export specs to align with the permanent sprint model and adjusted position/reordering behavior.
Reviewed changes
Copilot reviewed 60 out of 60 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/models/work_packages/pdf_export/work_package_to_pdf_spec.rb | Updates PDF export expectations (adds Story Points/Position/Sprint handling). |
| modules/backlogs/spec/services/work_packages/update_service_sprint_preservation_spec.rb | Removes feature-flag gating from sprint-preservation service specs. |
| modules/backlogs/spec/services/sprints/finish_service_spec.rb | Removes feature-flag gating from sprint finish service specs. |
| modules/backlogs/spec/routing/rb_stories_routing_spec.rb | Makes sprint story move route always available (no flag split). |
| modules/backlogs/spec/routing/rb_sprints_routing_spec.rb | Makes sprint dialog/start/finish routes always available (no flag split). |
| modules/backlogs/spec/routing/projects/settings/backlog_sharings_routing_spec.rb | Makes backlog sharing settings routes always available (no flag split). |
| modules/backlogs/spec/routing/inbox_routing_spec.rb | Removes feature-flag gating from inbox routing spec. |
| modules/backlogs/spec/requests/rb_master_backlogs_spec.rb | Aligns master backlogs request behavior with permanent backlog action and turbo-frame src params. |
| modules/backlogs/spec/requests/api/v3/sprints/show_resource_spec.rb | Removes API v3 sprints show feature-flag guard coverage. |
| modules/backlogs/spec/requests/api/v3/sprints/project_index_resource_spec.rb | Removes API v3 project sprints index feature-flag guard coverage. |
| modules/backlogs/spec/requests/api/v3/sprints/index_resource_spec.rb | Removes API v3 sprints index feature-flag guard coverage. |
| modules/backlogs/spec/models/work_packages/position_spec.rb | Removes feature-flag gating from position behavior specs. |
| modules/backlogs/spec/models/queries/work_packages/filter/sprint_filter_spec.rb | Updates sprint filter availability specs to no longer depend on flag state. |
| modules/backlogs/spec/lib/open_project/backlogs/permissions_spec.rb | Makes sprint-related permissions visible unconditionally (no flag split). |
| modules/backlogs/spec/lib/api/v3/work_packages/work_package_representer_rendering_spec.rb | Removes flag-dependent representer behavior branches for sprint/storyPoints/position. |
| modules/backlogs/spec/lib/api/v3/work_packages/schema/work_package_schema_representer_spec.rb | Removes flag-dependent schema branches for storyPoints/position/sprint. |
| modules/backlogs/spec/features/work_packages/sprints_on_wp_view_spec.rb | Removes feature-flag “off” scenario and makes sprint attribute always expected (subject to permissions). |
| modules/backlogs/spec/features/work_packages/filter_spec.rb | Removes sprint filtering feature-flag gating. |
| modules/backlogs/spec/features/work_packages/drag_in_sprint_spec.rb | Removes feature-flag gating from sprint drag/drop feature spec. |
| modules/backlogs/spec/features/work_packages/drag_in_inbox_spec.rb | Removes feature-flag gating from inbox drag/drop feature spec. |
| modules/backlogs/spec/features/work_packages/create_work_package_spec.rb | Removes feature-flag gating from create-in-sprint feature spec. |
| modules/backlogs/spec/features/projects/settings/backlog_sharing_settings_spec.rb | Removes feature-flag “tab hidden” scenario; sharing tab now depends on permission only. |
| modules/backlogs/spec/features/inbox_column_spec.rb | Removes feature-flag gating from inbox column feature spec. |
| modules/backlogs/spec/features/burndown/show_spec.rb | Removes feature-flag gating from burndown chart feature spec. |
| modules/backlogs/spec/features/backlogs/start_finish_spec.rb | Removes feature-flag gating from start/finish sprint feature spec. |
| modules/backlogs/spec/features/backlogs/sprint_list_spec.rb | Removes feature-flag gating from sprint list feature spec. |
| modules/backlogs/spec/features/backlogs/edit_spec.rb | Renames context and removes feature-flag gating in edit sprint planning spec. |
| modules/backlogs/spec/features/backlogs/create_spec.rb | Renames context and removes feature-flag “inactive” scenario in create spec. |
| modules/backlogs/spec/features/admin/backlogs_settings_spec.rb | Removes legacy admin configuration scenarios; asserts blankslate instead. |
| modules/backlogs/spec/controllers/rb_taskboards_controller_spec.rb | Removes flag split and consolidates taskboard controller expectations. |
| modules/backlogs/spec/controllers/rb_stories_controller_spec.rb | Removes flag split and keeps both Agile::Sprint and version Sprint behaviors covered. |
| modules/backlogs/spec/controllers/rb_sprints_controller_spec.rb | Removes flag gating for sprint dialogs/create/update/start/finish and adds refresh_form edit-mode case. |
| modules/backlogs/spec/controllers/projects/settings/backlog_sharings_controller_spec.rb | Removes feature-flag gating from backlog sharings controller spec. |
| modules/backlogs/spec/controllers/inbox_controller_spec.rb | Removes feature-flag gating from inbox controller spec. |
| modules/backlogs/lib/open_project/backlogs/work_package_filter.rb | Disables legacy backlogs-type filter availability by hard-coding configuration false. |
| modules/backlogs/lib/open_project/backlogs/patches/work_package_patch.rb | Removes legacy story/task/backlogs-type classification by hard-coding methods to empty/false; keeps sprint association/list behavior. |
| modules/backlogs/lib/open_project/backlogs/patches/versions/row_component_patch.rb | Removes backlogs-specific edit link injection; defers to upstream button links. |
| modules/backlogs/lib/open_project/backlogs/patches/type_patch.rb | Hard-codes Type#story?/task? to false (legacy classification removed). |
| modules/backlogs/lib/open_project/backlogs/patches/set_attributes_service_patch.rb | Always nullifies sprint when moved to a project without sprint visibility; removes legacy version inheritance logic. |
| modules/backlogs/lib/open_project/backlogs/patches/api/work_package_schema_representer.rb | Removes scrum feature decision from sprint schema visibility conditions. |
| modules/backlogs/lib/open_project/backlogs/patches/api/work_package_representer.rb | Removes scrum feature decision from sprint link/embed visibility conditions. |
| modules/backlogs/lib/open_project/backlogs/list.rb | Makes acts_as_list scoping consistently project+sprint and simplifies scope-change handling; uses rebuild service in nil-position edge case. |
| modules/backlogs/lib/open_project/backlogs/engine.rb | Makes sprint-related permissions and project menu entries always visible when module enabled; simplifies attribute constraints. |
| modules/backlogs/lib/api/v3/sprints/sprints_by_project_api.rb | Removes feature-flag guard from project sprints API. |
| modules/backlogs/lib/api/v3/sprints/sprints_api.rb | Removes feature-flag guard from global sprints API. |
| modules/backlogs/config/routes.rb | Removes feature-flag route constraints so Agile::Sprint routes are always mounted. |
| modules/backlogs/app/views/backlogs_settings/show.html.erb | Always renders Backlogs administration blankslate (legacy config form removed). |
| modules/backlogs/app/models/queries/work_packages/filter/sprint_filter.rb | Makes sprint filter availability depend only on permission (no feature decision). |
| modules/backlogs/app/helpers/rb_common_helper.rb | Removes scrum feature decision checks; disables legacy type lists and standardizes sprint board label. |
| modules/backlogs/app/forms/my/backlogs_form.rb | Removes legacy task color preference field gated by feature decision. |
| modules/backlogs/app/controllers/rb_taskboards_controller.rb | Switches “new vs legacy” behavior based on sprint class (Agile::Sprint vs Sprint). |
| modules/backlogs/app/controllers/rb_stories_controller.rb | Switches story loading behavior based on sprint class (Agile::Sprint vs Sprint). |
| modules/backlogs/app/controllers/rb_master_backlogs_controller.rb | Makes backlog view the default for index/details and always loads Agile::Sprint-based data. |
| modules/backlogs/app/controllers/rb_burndown_charts_controller.rb | Switches burndown creation based on sprint class (Agile::Sprint vs Sprint). |
| modules/backlogs/app/controllers/rb_application_controller.rb | Removes legacy “plugin configured” check and uses Agile::Sprint-first sprint loading with fallback to legacy Sprint. |
| modules/backlogs/app/controllers/projects/settings/backlogs_controller.rb | Always rebuilds positions via WorkPackages::RebuildPositionsService. |
| modules/backlogs/app/controllers/inbox_controller.rb | Removes feature-flag authorization gate for inbox actions. |
| modules/backlogs/app/components/projects/settings/backlogs/settings_header_component.html.erb | Shows sharing tab based on permission only (no feature decision). |
| config/initializers/feature_decisions.rb | Removes scrum_projects feature decision registration. |
| app/forms/versions/form.rb | Disables legacy version backlog settings UI by hard-coding backlogs_enabled? to false. |
Comments suppressed due to low confidence (1)
modules/backlogs/lib/open_project/backlogs/work_package_filter.rb:88
backlogs_configured?is now hard-coded tofalse, which makes this filter permanently unavailable (available?will always be false when backlogs is enabled). Since the filter still contains SQL logic for story/task/impediment, this looks like dead/stranded code. Consider removing the filter (and any API/schema exposure) entirely if it’s no longer part of the sprint-based Backlogs model, or update it to reflect the new model so it remains usable.
def backlogs_configured?
false
end
def backlogs_enabled?
project.nil? || project.module_enabled?(:backlogs)
end
modules/backlogs/lib/open_project/backlogs/patches/work_package_patch.rb
Outdated
Show resolved
Hide resolved
modules/backlogs/lib/open_project/backlogs/patches/work_package_patch.rb
Outdated
Show resolved
Hide resolved
8f370d4 to
f39b8b9
Compare
f39b8b9 to
d4064f4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 137 out of 137 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
modules/backlogs/app/services/work_packages/rebuild_positions_service.rb:55
- When
@projectis provided, the UPDATE is filtered, but themappingsubquery still computesROW_NUMBER()over the entirework_packagestable. On large instances this can be unnecessarily expensive. Consider applying the same project condition inside the subquery (e.g., in theFROM work_packagespart) so the window function only scans rows for the target project when@projectis set.
modules/backlogs/app/components/backlogs/sprint_menu_component.rb
Outdated
Show resolved
Hide resolved
modules/backlogs/spec/services/work_packages/rebuild_positions_service_integration_spec.rb
Show resolved
Hide resolved
modules/backlogs/spec/services/work_packages/rebuild_positions_service_integration_spec.rb
Show resolved
Hide resolved
modules/backlogs/app/controllers/rb_burndown_charts_controller.rb
Outdated
Show resolved
Hide resolved
|
@myabc copied from our chat:
The blankslate should be kept. The idea was to keep the route alive even though all the options were removed in 17.3, because new settings will be added soon.
I assume you are referring to the removal of the
It should definitely be removed ASAP. We don't rely on it any more. Whether the removal is part of this PR or the next, I don't care.
The change I could spot was adding the |
modules/backlogs/lib/open_project/backlogs/patches/work_package_patch.rb
Show resolved
Hide resolved
modules/backlogs/lib/open_project/backlogs/burndown/series_raw_data.rb
Outdated
Show resolved
Hide resolved
b0e84c4 to
70177ca
Compare
81e55e3 to
4633f58
Compare
| WorkPackage.transaction do | ||
| # Remove position from all non-stories | ||
| WorkPackage.where(["project_id = ? AND type_id NOT IN (?) AND position IS NOT NULL", project, Story.types]) | ||
| WorkPackage.where(project:) | ||
| .where.not(parent_id: nil) | ||
| .where.not(position: nil) | ||
| .update_all(position: nil) |
There was a problem hiding this comment.
rebuild_story_positions clears position for all child work packages in the given project (where.not(parent_id: nil)), regardless of whether they belong to this version/sprint. This can unexpectedly wipe ordering data for work packages in other versions/sprints (or inbox) within the same project. Consider scoping the cleanup to work_packages belonging to self (and/or to the relevant container key such as version_id/sprint_id) before rebuilding positions, instead of operating on the whole project.
Make Backlogs use the sprint-based behavior unconditionally and remove the old feature-flagged branches from controllers, routes, representers, and supporting helpers. Update the affected Backlogs specs and PDF export expectations to match the permanent sprint model and keep list reordering stable when moving work packages between backlog and sprint scopes. https://community.openproject.org/wp/73798
Restore the minimal admin settings blankslate so the admin menu route remains valid after the sprint-based cleanup. Remove the remaining settings-driven story/task classification code, dead models and services, and the obsolete filter and spec setup that depended on it.
479fa5b to
d10c474
Compare
| context "when not an admin" do | ||
| let(:user) { create(:user) } | ||
|
|
||
| it "fails" do | ||
| it "requires admin" do | ||
| get :show | ||
| expect(response).to have_http_status :forbidden | ||
| end | ||
| end | ||
| end | ||
|
|
||
| describe "PUT update" do | ||
| before do | ||
| allow(Setting).to receive(:plugin_openproject_backlogs=) | ||
| end | ||
|
|
||
| subject do | ||
| put :update, | ||
| params: { | ||
| settings: { | ||
| task_type:, | ||
| story_types: | ||
| } | ||
| } | ||
| end | ||
|
|
||
| context "with invalid settings (Regression test #35157)" do | ||
| let(:task_type) { "1234" } | ||
| let(:story_types) { ["1234"] } | ||
|
|
||
| it "does not update the settings" do | ||
| subject | ||
|
|
||
| expect(response).to render_template "show" | ||
| expect(flash[:error]).to start_with I18n.t(:notice_unsuccessful_update_with_reason, reason: "") | ||
|
|
||
| expect(Setting).not_to have_received(:plugin_openproject_backlogs=).with(any_args) | ||
| end | ||
| end | ||
|
|
||
| context "with valid settings" do | ||
| let(:task_type) { "1234" } | ||
| let(:story_types) { ["5555"] } | ||
|
|
||
| it "does update the settings" do | ||
| subject | ||
|
|
||
| expect(response).to redirect_to action: :show | ||
| expect(flash[:notice]).to include I18n.t(:notice_successful_update) | ||
| expect(flash[:error]).to be_nil | ||
|
|
||
| expect(Setting).to have_received(:plugin_openproject_backlogs=).with( | ||
| points_burn_direction: nil, | ||
| story_types: [5555], | ||
| task_type: 1234, | ||
| wiki_template: nil | ||
| ) | ||
| end | ||
|
|
||
| context "with a non-admin" do | ||
| current_user { build_stubbed(:user) } | ||
|
|
||
| it "does not update the settings" do | ||
| subject | ||
|
|
||
| expect(response).not_to be_successful | ||
| expect(response).to have_http_status :forbidden | ||
|
|
||
| expect(Setting).not_to have_received(:plugin_openproject_backlogs=).with(any_args) | ||
| end | ||
| expect(response).not_to have_http_status(:ok) |
There was a problem hiding this comment.
In this spec, the non-admin case only asserts that the response is not :ok. That’s very permissive and can mask regressions (e.g., redirect vs. forbidden vs. error). Consider asserting the expected behavior of require_admin explicitly (e.g., :forbidden / redirect to login) to make the test meaningful.
Drop the unused story, task, and wiki template plugin settings and the legacy classification hooks that still read them. Rebuild Backlogs positions from the current parent/child structure, remove the obsolete Sprint wiki helpers, and keep points_burn_direction as the only remaining plugin setting.
Restore `#backlog` as the canonical Backlogs route by redirecting all `#index` requests to it.
Deletes the jQuery Taskboard Sass and JS chain. Keeps the Backlogs task board route and links redirecting into Boards unchanged.
Add `clear_filter_value` to clear an ng-select filter's current selection without removing the filter row, allowing a new value to be set in place. Refactor `insert_autocomplete_item` to accept a filter `id` string rather than a pre-captured element, so the ng-select node is re-queried on each call. This avoids stale element references after `clear_filter_value` causes the DOM to re-render.
d10c474 to
cc6b272
Compare
Ticket
https://community.openproject.org/wp/73798
What are you trying to accomplish?
Remove the
scrum_projectsfeature flag from Backlogs and make the sprint-based implementation permanent.What approach did you choose and why?
The flag was already permanently active, so the old branches were dead code. This PR:
scrum_projectsfeature registration and the remaining flag checksrb_taskboardsredirect endpoint so sprint links continue to resolve to the linked boardScreenshots
Merge checklist