Project

General

Profile

Actions
Copy link

Defect #29838

closed

Time logging via commit message does not work when the configured activity has been overridden on the project level

Added by Jens Krämer about 5 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Go MAEDA
Category:
Time tracking
Target version:
4.2.0
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

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.

Files

Download all files
0001-fix-logging-time-via-a-commit-message-for-project-sp.patch (4.08 KB) 0001-fix-logging-time-via-a-commit-message-for-project-sp.patch revised and squashed patch Jens Krämer, 2020-02-11 07:45
0002-Fix-Rubocop-offense-Rails-FindBy-Use-find_by-instead.patch (829 Bytes) 0002-Fix-Rubocop-offense-Rails-FindBy-Use-find_by-instead.patch Marius BALTEANU, 2020-02-11 09:20
Actions
Copy link
 #1

Updated by Jens Krämer about 5 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

Actions
Copy link
 #2

Updated by Go MAEDA about 5 years ago

  • Target version set to Candidate for next major release
Actions
Copy link
 #3

Updated by Go MAEDA about 5 years ago

  • Target version changed from Candidate for next major release to 4.1.0
Actions
Copy link
 #4

Updated by Go MAEDA over 4 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
Actions
Copy link
 #5

Updated by Go MAEDA about 4 years ago

  • Target version changed from 4.1.0 to Candidate for next major release

Removing from 4.1.0 because the test fails.

Actions
Copy link
 #6

Updated by Jens Krämer almost 4 years ago

I just applied both patches to current master and all changeset tests pass. Could you please retry / post some logs?

Actions
Copy link
 #7

Updated by Marius BALTEANU almost 4 years ago

  • File deleted (0002-small-refactoring-moves-the-method-to-find-the-activ.patch)
Actions
Copy link
 #8

Updated by Marius BALTEANU almost 4 years 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

Actions
Copy link
 #9

Updated by Jens Krämer almost 4 years 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?

Actions
Copy link
 #10

Updated by Marius BALTEANU almost 4 years ago

Jens Krämer wrote:

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?

Yes, you can see in .gitlab-ci.yml:

I use MySQL 5.7 because the same version is used also by the CI system on redmine.org, see Continuous_integration.

Actions
Copy link
 #11

Updated by Jens Krämer almost 4 years 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.

Actions
Copy link
 #12

Updated by Marius BALTEANU almost 4 years ago

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

Actions
Copy link
 #13

Updated by Marius BALTEANU almost 4 years ago

  • File deleted (0002-small-refactoring-moves-the-method-to-find-the-activ.patch)
Actions
Copy link
 #14

Updated by Marius BALTEANU almost 4 years ago

  • File deleted (0001-fix-logging-time-via-a-commit-message-for-project-sp.patch)
Actions
Copy link
 #15

Updated by Go MAEDA almost 4 years ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA

Committed the patches. Thank you for detecting and fixing this issue.

Actions
Copy link
 #16

Updated by Marius BALTEANU over 2 years ago

  • Subject changed from time logging via commit message does not work when the configured activity has been overridden on the project level to Time logging via commit message does not work when the configured activity has been overridden on the project level
Actions
Copy link
 #17

Updated by Go MAEDA over 2 years ago

  • Tracker changed from Patch to Defect
  • Resolution set to Fixed
Actions
Copy link

Also available in: Atom PDF