Project

General

Profile

Actions

Defect #13487

closed

Honor committer => user mapping in repository statistics

Added by Anonymous about 11 years ago. Updated over 9 years ago.

Status:
Closed
Priority:
Normal
Category:
SCM
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

Description

This patch changes the repository statistics view to honor the "commiter => user" mapping. Without it, the statistics page is pretty much useless for it.

After creating this patch, i discovered issue #2624 which is about the same issue, and also provides a patch; unfortunately it has been mostly ignored for 4 years.

I still decided to submit my own patch, because it has some advantages:
  • It properly deals with the case were a committer name has not been mapped to a user. The patch at issue #2624 seems to map such committers to the empty string.
  • It sorts case insensitive, which is quite useful for me.
  • It merges homonyms, as requested by #2065. Actually, this point could also be a drawback for some projects, namely if two distinct people sharing the same name work there. So perhaps this is not desired. I can change the patch to remove this, which will then only merge committers mapping to the same user.

Files


Related issues

Related to Redmine - Feature #2624: Repository statistics should honour user-mappingClosed2009-01-30

Actions
Related to Redmine - Feature #2065: Commit graph should merge homonymsClosedJean-Baptiste Barth

Actions
Related to Redmine - Feature #2761: Factorize user name in /repository/statisticsClosedJean-Baptiste Barth2009-02-16

Actions
Related to Redmine - Defect #14338: svn: repository statistics issue when user names only differ in caseClosedJean-Baptiste Barth

Actions
Has duplicate Redmine - Feature #7353: Map repository authors to Redmine users not used in statisticsClosed2011-01-17

Actions
Actions #1

Updated by Anonymous about 11 years ago

This patch also fixes issues #7353 and #2761 (which are duplicates of other issues, but nobody flagged them as such, and I don't seem to be allowed to flag issues as duplicates).

Actions #2

Updated by Etienne Massip about 11 years ago

  • Category set to SCM
Actions #3

Updated by Daniel Felix about 11 years ago

Well I would like to test this issue. But currently I'm at the hospital. Just in case of the description, this sounds to be worth to look at it. :-)

Actions #4

Updated by Anonymous about 11 years ago

No worries. Get back to good health and return home from hospital, redmine is no nearly as important as that!

I just hope this patch won't share the fate of those others and still be here, unresolved (i.e. neither accepted nor rejected) in four years g.

Actions #5

Updated by Daniel Felix about 11 years ago

Thanks!

Well I think this one will find a faster way. If there is the next minor release available this could be set to this release or at least the next major.

Actions #6

Updated by Anonymous almost 11 years ago

Sadly after 1 month, it seems nobody had a chance to look at this. Too bad :-(. It would be nice to get at least some feedback on these things... as it is, development of Redmine seems to happen mostly behind closed doors and is not very welcome to outside contributions :-(.

Perhaps somebody did look at this patch and found an issue with that -- but without any feedback, that's impossible to tell for me :-(.

Actions #7

Updated by Toshi MARUYAMA almost 11 years ago

Because this patch does not have tests, I don't know it is correct or not.
I think it is better that Graph logic is moved from controller to helper in order to test.
See: source:tags/2.3.0/test/unit/lib/redmine/helpers/gantt_test.rb

Actions #8

Updated by Anonymous almost 11 years ago

OK, that makes sense. I am not yet very familiar with the testing system (of Redmine, or any other Ruby / RoR app in general -- I am fairly new to Ruby). I will investigate the gantt_test you referred me to, thanks for the point. I will try and see if I can do something. Likewise, moving the logic from controller to helper is something I can do, if you prefer that.

However, currently I am swamped with other work, so it's easily possible that another month or two will pass before I get to address either point. If somebody else beats me to that with a better patch, I won't complain ;-) but in any case, the ball is now in my court, as I have some clear feedback. Thank you very much for that, and for your work on Redmine in general.

Actions #9

Updated by Keats . about 10 years ago

Hi,
it's working great.
Thanks

Actions #10

Updated by txemi M almost 10 years ago

I would love to see this patch in next version,
thanks.

Actions #11

Updated by Jean-Baptiste Barth over 9 years ago

  • Related to Defect #14338: svn: repository statistics issue when user names only differ in case added
Actions #12

Updated by Jean-Baptiste Barth over 9 years ago

  • Tracker changed from Patch to Defect
  • Assignee set to Jean-Baptiste Barth
  • Target version set to 2.6.0

I started looking at it and adding some tests, I'll commit it soon, hopefully for an integration in 2.6.0 (it's a fix
but it has been here for a very long time, so it can wait past 2.5.3).

Maxin Samuel: thanks for the proposals but I started again from scratch because, as Toshi said, this code should'nt be here (some code belongs to the model imho, some other to helpers for SVG rendering). Moreover your patch has many effects in one and I'd like to keep things in separate commits so Toshi or Jean-Philippe can easily revert this if they judge it's not correct. Anyway it helped me see where the problem is so thanks for your contribution !

Actions #13

Updated by Jean-Baptiste Barth over 9 years ago

  • Related to deleted (Feature #7353: Map repository authors to Redmine users not used in statistics)
Actions #14

Updated by Jean-Baptiste Barth over 9 years ago

  • Has duplicate Feature #7353: Map repository authors to Redmine users not used in statistics added
Actions #15

Updated by Toshi MARUYAMA over 9 years ago

Actions #16

Updated by Jean-Baptiste Barth over 9 years ago

Toshi MARUYAMA wrote:

Ruby 1.8.7 PostgreSQL test fails.
https://travis-ci.org/marutosi/redmine-bb/jobs/33601888

Thanks! That should be fixed in r13352. There's no check on Change#action length as it's not user-controlled but it seems SQL drivers don't react the same way so we may be better adding some...

Actions #17

Updated by Jean-Baptiste Barth over 9 years ago

I added committers/users mapping in r13353, but I did not add the other features discussed by Max in the original issue (sort committer names, merge homonyms). In case tests pass, and if JPL and Toshi are OK with that, this feature could be released with Redmine 2.6.0.

Note that this can have a bit of performance impact because with this implementation multiple queries are run to find the correct user (it relies on Repository#find_committer_user while it could use Changeset#user_id directly). I'll improve that in the next few days.

Also it made me realize there are missing tests for Repository#find_committer_user and I think some edge cases are not handled correctly. I'll dig into that later.

Actions #18

Updated by Toshi MARUYAMA over 9 years ago

Toshi MARUYAMA wrote:

Ruby 1.8.7 PostgreSQL test fails.
https://travis-ci.org/marutosi/redmine-bb/jobs/33601888

Because I restarted, log disappeared.
Ruby 1.9.3 PostgreSQL test fails due to same reason.
https://travis-ci.org/marutosi/redmine-bb/jobs/33601897

  1) Error:
test_stats_by_author_reflect_changesets_and_changes(RepositoryTest):
ActiveRecord::StatementInvalid: PG::StringDataRightTruncation: ERROR:  value too long for type character varying(1)
: INSERT INTO "changes" ("action", "branch", "changeset_id", "from_path", "from_revision", "path", "revision") VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING "id" 

Actions #19

Updated by Jean-Baptiste Barth over 9 years ago

Fwiw here's the patch to cut down the number of SQL queries by relying directly on Changeset#user_id. On my test repo (10k changesets, 40k file changes, 90 distinct committers), it cuts the number of SQL queries from 216 to 9.

But I don't commit it now because I'd like to have the feedback of the CI system which is down until the end of the week. I don't know if Oracle and SQLServer will support the custom select forms and I didn't see "AS" elsewhere in the codebase, so I want to be able to revert it quickly if it breaks.

Actions #20

Updated by Jean-Baptiste Barth over 9 years ago

  • Status changed from New to Closed
  • Resolution set to Fixed
  • Affected version set to 2.5.2

All tests passing with r13361/r13362, so I close this issue. Other features deserve their own issue (but as said elsewhere I doubt they're a good idea in the general case for now ; maybe someday when we have something prettier client-side).

Actions

Also available in: Atom PDF