Replace the unicode toggle with Redmine's sprite icon - #44
Closed
mboremski wants to merge 16 commits into
Closed
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
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.
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.
Fixes xmera-circle#3 The trigger used the unicode characters U+25B6 / U+25BC, which render differently across browsers and platforms and do not follow the theme. Redmine 6 ships an icon sprite; 'angle-right' is the shape the core uses for the same purpose. The icon is now rendered once in the partial and the state is expressed purely by the existing css class: `.trigger.opened svg` is rotated by 90 degrees. JavaScript no longer rewrites the element's content, so the class is the single source of truth. That also fixes a latent inconsistency: when a level was collapsed, the nested triggers below it had their glyph replaced but kept the `opened` class. They now get `closed` like the element the user actually clicked, so a subsequent click on them behaves as expected instead of collapsing something that already looks collapsed. `slides.js` shrinks by four lines and no longer knows about presentation.
mboremski
marked this pull request as ready for review
August 27, 2026 05:35
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
2 participants
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.
Fixes #3
The trigger used the unicode characters U+25B6 / U+25BC, which render
differently across browsers and platforms and do not follow the theme.
Redmine 6 ships an icon sprite; 'angle-right' is the shape the core uses
for the same purpose.
The icon is now rendered once in the partial and the state is expressed
purely by the existing css class:
.trigger.opened svgis rotated by 90degrees. JavaScript no longer rewrites the element's content, so the
class is the single source of truth.
That also fixes a latent inconsistency: when a level was collapsed, the
nested triggers below it had their glyph replaced but kept the
openedclass. They now get
closedlike the element the user actually clicked,so a subsequent click on them behaves as expected instead of collapsing
something that already looks collapsed.
slides.jsshrinks by four lines and no longer knows about presentation.