Get 4.0.0 tested on Redmine 6.1: CI, test fixes, version declaration - #43
Merged
Conversation
Wrap the `to_date` conversion in `sanitizeDateParameter` with a begin-rescue block to catch `Date::Error`. Previously, strings like "2026-01-32" passed the `respond_to?(:to_date)` check but raised an unhandled exception during conversion. Now invalid dates fall back to the default value instead of crashing. Adds functional tests for all three date parameters (first_day, last_day, start_date) to cover the invalid date case. Fixes xmera-circle#41 https://claude.ai/code/session_01BSqVnAYvK1kCejeMnp4MqM
Fix HTTP 500 error when invalid date is entered in workload filter
When the user sets a 'Use as today' (start_date) value beyond the last_day of the displayed time span, the UserWorkload model crashes with a NoMethodError because 'today' falls outside the time_span and hours_for_issue[today] returns nil. Cap @today to @last_day in the controller before passing it to UserWorkload. Show a flash warning when the capping occurs so the user is informed. Fixes xmera-circle#40 https://claude.ai/code/session_01BSqVnAYvK1kCejeMnp4MqM
Fix HTTP 500 when 'Use as today' date is after end date
Collaborator
Author
|
We are working on upgrading our instance to redmine6.1 |
The 4.x line requires Redmine 6: migrations were bumped to
ActiveRecord::Migration[7.2] in 4.0.0, which Rails 6.1 (Redmine 5.0)
rejects with `Unknown migration version "7.2"`. The workflow therefore
could not pass against 5.0-stable any more.
Changes:
* Rename 5-0-stable.yml to 6-1-stable.yml and export
branches/6.1-stable instead of 5.0-stable.
* Bump Ruby to 3.2. Redmine 6.1 requires `ruby '>= 3.2.0', '< 3.5.0'`,
so `bundle install` fails on 3.1.
* Add `rake assets:precompile` after the migrations. Redmine 6 uses
Propshaft.
* actions/checkout v3 -> v4.
Workflow name ("GitHub CI") and job id (plugin_tests) are unchanged so
existing required status checks keep matching. Redmine 5 remains served
by the 3.x line.
README: point the CI badge at the renamed workflow.
`rake redmine:plugins:test:units` runs the tests through
`Rails::TestUnit::Runner.run_from_rake`, which spawns `bin/rails test` in
a separate process. That process requires the test files *before* the
Rails environment is loaded, and on Rails 7.2 `rails/commands` no longer
pulls in railties' `rails.rb` on the way. `Rails` is then only a bare
namespace module and `Rails.root` does not exist:
test/test_helper.rb:4:in `<top (required)>':
undefined method `root' for Rails:Module (NoMethodError)
On Rails 6.1 (Redmine 5.0) `rails.rb` happened to be loaded by then, so
the same line worked. Resolve Redmine's test_helper by path instead,
which is what Redmine's own plugins do and what the environment does not
have to be loaded for.
plugins/<plugin>/test/test_helper.rb -> ../../../test/test_helper
The plugin test tasks print the failure but not the surrounding log. Attaching test.log as an artifact on failure makes the next iteration diagnosable without reproducing locally. Taken from the DMSF workflow.
Two tests failed with exactly one extra user: WlUserSelectionTest#test_should_return_all_users_if_the_current_user_is_admin WlUserSelectionTest#test_should_return_all_active_users_when_user_has_permission_:view_all_workloads Redmine 6.0 added `fixtures :all` to ActiveSupport::TestCase in test/test_helper.rb; Redmine 5.0 did not have it. Every fixture file is now loaded no matter what a test class declares, including groups_users.yml, which makes users(:users_008) a member of groups 10 and 11. `WlUserSelection#all_users` is `User.joins(:groups).distinct.active` -- every user belonging to any group. Under Redmine 6 that legitimately includes users(:users_008), so comparing against only the users generated in setup is wrong. Confirmed in the CI log: WlUserData Load ... WHERE `wl_user_datas`.`user_id` IN (8, 41, 42, 43) The plugin behaves correctly; only the tests' assumption about the fixture universe was stale. Record the pre-existing group members in setup and add them to the expectation, walking Group#users rather than reusing the scope under test. Note that the per-class `fixtures ...` declarations throughout this plugin's test suite are effectively inert on Redmine 6 for the same reason. They are left alone here.
4.0.0 announced Redmine 6.0.x, but nothing in the repository ever tested against Redmine 6 and the plugin had no `requires_redmine` at all, so an incompatible install failed with a stack trace instead of a clear message. * init.rb: add `requires_redmine version_or_higher: '6.1'`. * README/CHANGELOG: state 6.1.z, the version the CI workflow covers. Redmine 6.0.z ships the same Rails 7.2 and probably works, but it is untested and therefore not claimed. * README: add a "support of Redmine 6" section spelling out that 4.x is not backward compatible (Migration[7.2]) and that Redmine 5 stays on the 3.x line. * README: the PostgreSQL/Ruby 3.1 warning is moot now that Redmine 6.1 requires Ruby >= 3.2; keep the note but say so.
With `requires_redmine >= 6.1` the plugin always runs on Rails 7.2 and Ruby >= 3.2, which makes several branches unreachable: * init.rb: the `Rails.version < '6'` block added `app/presenters` to `autoload_paths`. That directory does not exist in this repository, so the branch was dead even on Rails 5. * user_patch.rb: the `to_prepare` fallback for Rails < 6. The after_plugins_loaded hook does this on every supported version. * after_plugins_loaded_hook.rb: the Rails >= 6 guard is now always true. * init.rb / lib/redmine_workload.rb: the PostgreSQL Ruby-version guard and `RedmineWorkload.postgresql?`, its only caller. Redmine 6.1 requires Ruby >= 3.2, so the guard can never fire. If a PostgreSQL specific check is needed again, reinstate it with a real reason. No behaviour change on Redmine 6.
The two bug fixes and the two test fixes were not listed yet. Keeps the 4.0.0 section a complete description of what changed since 3.0.2. The 4.0.0 date still reads 2026-02-06 and should be updated when the version is actually tagged.
Collaborator
Author
|
@liaham shall I wait with a merge until you had a chance to do a review? |
Redmine 6 renders icons as SVG sprites. The plain `icon icon-add` class still works through the legacy fallback (`.icon-add:not(:has(svg))` puts a PNG behind it), but the result sits next to the core's SVG icons and looks out of place. Wrap the labels in `sprite_icon`, keeping the existing css classes, the same way the core views do (compare app/views/calendars/show.html.erb for the `link_to_function` case). All three sprite names -- add, checked, warning -- exist in Redmine 6.1's icons.svg.
The config still targeted Ruby 2.7. Redmine 6.1 requires `ruby '>= 3.2.0', '< 3.5.0'`, so 3.2 is the lowest version this plugin can actually run on. Rubocop is not part of the CI workflow, so this only affects local runs. Raising the target may surface new offences; .rubocop_todo.yml was left untouched.
RedmineToolbarHookListener injected slides.js and style.css into the
layout head of *every* Redmine page, not just the workload views. Two
consequences:
* every page in the installation paid for two extra requests it has no
use for
* style.css contained an unscoped `legend { color: ... }` rule, so the
plugin silently restyled the legend of every fieldset in Redmine --
including core pages and whatever the active theme intended
Restrict the hook to the four controllers whose views need the assets,
and scope the legend rule to `.controller-workloads`. `#year-nav` is an
id used only by this plugin's holiday view and is left as is.
The plugin settings page (`settings` controller) renders with core
classes only -- box, tabular, info -- and does not need style.css.
Member
|
@mboremski: Please move on and review and merge by yourself. Unfortunately, I have no ressources to assist you. |
Collaborator
Author
|
Thanks for your trust. Wikl move on. |
Merged
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
develophad no CI coverage for Redmine 6 at all — the workflow ran againstRedmine 5.0-stable, where the
ActiveRecord::Migration[7.2]migrations from #42cannot load. This PR points the CI at Redmine 6.1-stable, fixes what the tests
then found, and cleans up three things that only became visible once the plugin
actually ran on Redmine 6. (#38)
All three database engines are green. The plugin has also been verified by hand
against a production dataset (~2300 projects, 56k issues) on Redmine 6.1 with
the Opale theme: absences, redistribution of planned hours, team aggregation,
threshold colours, cross-group visibility, CSV export, and the full create /
edit / delete cycle on vacations and holidays.
Also contains two bug fixes that were already on this branch (#40, #41).
CI
5-0-stable.ymlto6-1-stable.ymland switched the Redmine exportto
branches/6.1-stable. The 4.x line requires Redmine 6 since Updates to support Redmine 6 and Rails 7 #42 moved themigrations to
ActiveRecord::Migration[7.2], which Rails 6.1 rejects withArgumentError: Unknown migration version "7.2". A 5.0 job can no longerpass; Redmine 5 is served by the 3.x line.
ruby '>= 3.2.0', '< 3.5.0', sobundle installfailed on 3.1.rake assets:precompile, Redmine 6 uses Propshaft.subversioninstall step;svnis no longer preinstalledon the runner image.
actions/checkoutv3 → v4.test.logis uploaded as an artifact when a job fails, so a red run can bediagnosed without reproducing it locally. Taken from the DMSF workflow.
The workflow name (
GitHub CI) and job id (plugin_tests) are unchanged, soexisting required status checks keep matching. Only the file name changed —
please check whether any branch protection rule refers to the old name.
Test fixes
Rails.rootat load time.redmine:plugins:test:*runs throughRails::TestUnit::Runner.run_from_rake, which spawnsbin/rails testas aseparate process; that process requires the test files before the Rails
environment is loaded. On Rails 7.2
rails/commandsno longer pulls inrailties'
rails.rbon the way, soRailsis a bare namespace module andRails.rootdoes not exist. Resolved by path instead, the way Redmine's ownplugins do. Works on Rails 6.1 as well.
fixtures :all. TwoWlUserSelectionTesttests failed with exactly oneextra user. Redmine 6.0 added
fixtures :alltoActiveSupport::TestCase, sogroups_users.ymlis always loaded andusers(:users_008)is a member of twogroups.
WlUserSelection#all_usersisUser.joins(:groups).distinct.active,so that user legitimately belongs in the result — the plugin is correct, the
expectation was stale. The test now records the pre-existing group members in
setup, walkingGroup#usersrather than reusing the scope under test.Worth knowing beyond these two tests: the per-class
fixtures ...declarations across this plugin's test suite are inert on Redmine 6. Other
tests that quietly assume a small fixture universe may show the same effect.
Left alone in this PR.
Version declaration
requires_redmine version_or_higher: '6.1'. The plugin had norequires_redmineat all, so an incompatible install failed with a stacktrace instead of a clear message.
made in Updates to support Redmine 6 and Rails 7 #42 — Redmine 6.0.z ships the same Rails 7.2 and probably works, but
nothing tests it. Happy to add
6.0-stableas a second matrix axis insteadand keep the wider claim; your call.
backward compatible and that Redmine 5 stays on the 3.x line.
describes everything that changed since 3.0.2. The date still reads
2026-02-06and should be updated when the version is tagged.Redmine 6 housekeeping
through Redmine 6's legacy fallback (
.icon-add:not(:has(svg))puts a PNGbehind them), but sit oddly next to the core's SVG icons and do not follow the
theme colour. Wrapped in
sprite_icon, css classes unchanged, the same waythe core views do.
RedmineToolbarHookListenerinjected
slides.jsandstyle.cssinto the layout head of every Redminepage. That was not just overhead —
style.csscontained an unscoped rule:The plugin therefore restyled the legend of every fieldset in the whole
installation, including core pages, overriding whatever the active theme
intended. The hook is now restricted to the plugin's four controllers and the
rule is scoped to
.controller-workloads.#year-navis an id used only bythis plugin's holiday view and is left as is. The plugin settings page renders
with core classes only (
box,tabular,info) and does not needstyle.css.Verified on a Redmine 6.1 instance: expanding and collapsing the hierarchy
still works, the plugin's assets appear in the head on
/workloadand nolonger on core pages.
.rubocop.ymltargeted Ruby 2.7; raised to 3.2, the lowest versionRedmine 6.1 allows. Rubocop is in no Gemfile and does not run in CI, so this
only affects local runs.
.rubocop_todo.ymluntouched.Dead code
Removed branches that cannot execute on Redmine 6:
Rails.version < '6'block ininit.rb— it added anapp/presentersdirectory that does not exist in this repository, so it was dead even on
Rails 5
to_preparefallback inuser_patch.rbafter_plugins_loaded_hook.rb, now always trueRedmineWorkload.postgresql?. The guardraised when PostgreSQL was combined with Ruby < 3.1 (the workday settings
change in 3.0.0, setting for general_workday_sunday replaced by workload_of_parent_issues #27). Redmine 6.1 requires Ruby >= 3.2, so it can never fire,
and the method had no other caller. PostgreSQL support itself is untouched and
still covered by the CI matrix.
No behaviour change on Redmine 6.
Bug fixes already on this branch
Wrap the
to_dateconversion insanitizeDateParameterwith a begin-rescueblock to catch
Date::Error. Previously, strings like "2026-01-32" passed therespond_to?(:to_date)check but raised an unhandled exception duringconversion. Now invalid dates fall back to the default value instead of
crashing. Adds functional tests for all three date parameters (first_day,
last_day, start_date) to cover the invalid date case.
Fixes #41
When the user sets a 'Use as today' (start_date) value beyond the last_day of
the displayed time span, the UserWorkload model crashes with a NoMethodError
because 'today' falls outside the time_span and hours_for_issue[today] returns
nil. Cap @today to @last_day in the controller before passing it to
UserWorkload. Show a flash warning when the capping occurs so the user is
informed.
Fixes #40
Both were re-verified by hand on Redmine 6.1.
Open points for you
4.0.0CHANGELOG date, to be set when the version is tagged.fixtures ...declarations across the test suite — separatecleanup, happy to do it if you want it.