Patch #29838
time logging via commit message does not work when the configured activity has been overridden on the project level
Status: | Closed | Start date: | ||
---|---|---|---|---|
Priority: | Normal | Due date: | ||
Assignee: | % Done: | 0% | ||
Category: | Time tracking | |||
Target version: | 4.2.0 |
Description
This happened with a client at Planio, we came up with the attached patch which fixes the problem by looking up the activity in the activities of the associated issues' project. Test case included to illustrate the problem.
The second patch is just a small refactoring which moves the activity finder to the Project model.
Associated revisions
Fix logging time via a commit message for project specific activities (#29838).
Patch by Jens Krämer.
History
#1
Updated by Jens Krämer about 2 years ago
- File 0002-small-refactoring-moves-the-method-to-find-the-activ.patch added
updated version of the second patch, better method name to match the setting
#2
Updated by Go MAEDA about 2 years ago
- Target version set to Candidate for next major release
#3
Updated by Go MAEDA about 2 years ago
- Target version changed from Candidate for next major release to 4.1.0
#4
Updated by Go MAEDA almost 2 years ago
- Status changed from New to Needs feedback
The test fails in my environment.
Failure: ChangesetTest#test_ref_keywords_any_with_timelog [test/unit/changeset_test.rb:142]: Expected false to be truthy. bin/rails test test/unit/changeset_test.rb:106
#5
Updated by Go MAEDA over 1 year ago
- Target version changed from 4.1.0 to Candidate for next major release
Removing from 4.1.0 because the test fails.
#6
Updated by Jens Krämer 11 months ago
I just applied both patches to current master and all changeset tests pass. Could you please retry / post some logs?
#7
Updated by Marius BALTEANU 11 months ago
- File deleted (
0002-small-refactoring-moves-the-method-to-find-the-activ.patch)
#8
Updated by Marius BALTEANU 11 months ago
Jens Krämer wrote:
I just applied both patches to current master and all changeset tests pass. Could you please retry / post some logs?
I've added your patches to my Gitlab CI instance used by me to run the tests and it fails only on MySQL: https://gitlab.com/redmine-org/redmine/-/jobs/431381873
Also, the patches introduce a new Rubocop violation that should be fixed: https://gitlab.com/redmine-org/redmine/-/jobs/431381872
You can see the pipeline here: https://gitlab.com/redmine-org/redmine/pipelines/116431745
#9
Updated by Jens Krämer 11 months ago
thats really interesting as I ran my tests against MySQL (5.6) as well. can i see the exact environment in terms of ruby / mysql version somewhere on gitlab?
#10
Updated by Marius BALTEANU 11 months ago
Jens Krämer wrote:
Yes, you can see inthats really interesting as I ran my tests against MySQL (5.6) as well. can i see the exact environment in terms of ruby / mysql version somewhere on gitlab?
.gitlab-ci.yml
:
- ruby version: https://gitlab.com/redmine-org/redmine/-/blob/feature/29838/.gitlab-ci.yml#L1
- mysql version: https://gitlab.com/redmine-org/redmine/-/blob/feature/29838/.gitlab-ci.yml#L18
- postgres version: https://gitlab.com/redmine-org/redmine/-/blob/feature/29838/.gitlab-ci.yml#L26
I use MySQL 5.7 because the same version is used also by the CI system on redmine.org, see Continuous_integration.
#11
Updated by Jens Krämer 11 months ago
turns out it did not have anything to do with the MySQL version, it was just a race condition in the Changeset test suite due to the commit_logtime_activity_id
setting sometimes being set from a previous test case. The attached combined patch fixes that and replaces all previous patches.
#12
Updated by Marius BALTEANU 11 months ago
- File 0002-Fix-Rubocop-offense-Rails-FindBy-Use-find_by-instead.patch
added
- Status changed from Needs feedback to New
- Target version changed from Candidate for next major release to 4.2.0
Jens Krämer wrote:
turns out it did not have anything to do with the MySQL version, it was just a race condition in the Changeset test suite due to the
commit_logtime_activity_id
setting sometimes being set from a previous test case. The attached combined patch fixes that and replaces all previous patches.
All tests pass now with the attached patch that fixes a Rubocop offense: https://gitlab.com/redmine-org/redmine/pipelines/116780713
#13
Updated by Marius BALTEANU 11 months ago
- File deleted (
0002-small-refactoring-moves-the-method-to-find-the-activ.patch)
#14
Updated by Marius BALTEANU 11 months ago
- File deleted (
0001-fix-logging-time-via-a-commit-message-for-project-sp.patch)