Implement contributors graph #27882
Implement contributors graph #27882
Conversation
Co-authored-by: silverwind <me@silverwind.io>
|
09d5d27 is probably breaking my code. It should not generate sundays (or start dates) based on user's local time zone because sometimes some countries change their clock and this will cause all the generated timestamps to be shifted. Then, it won't match the timestamps generated from backend, and will result in empty spaces in graphs:
I will mark as 'ready for review' again once I fix this. |
Hmm I did carefully check that the function's output is the same before and after the change, are you sure? We should add a small unit test for this function to ensure correct output, I can do that later. |
yeah, I am sure. It might have worked because maybe in your country, you don't advance clocks to save day time. Here is what I am talking about: https://en.wikipedia.org/wiki/Daylight_saving_time |
|
Okay, sorry for that. I will add a unit test to the function in a bit. The comment you added seems a bit hostile, and I think instead of such comments, we should just have proper unit tests that confirm the functionality. |
No, I didn't want to be mean. I am sorry if it sounds like that. As most of the people here, english is not my main language so I guess that's the problem. I didn't want to offend anyone in any comment of mine. Yes, a unit test might be good 👍 Would you like to add it? |
|
Yes I will add it. It's likely a good use case for a snapshot test. |
|
I moved the three functions and added a simple test. As I understand it, that test should succeed when ran in every time zone because start and end day are Thursdays. |
Nope, testing against thursdays is not different from testing against sundays. Backend code generates timestamps with UTC+0 and it will never advance clock for 1 hour. (i.e. There will be always 168 hours between two consecutive sundays, or tuesdays or thursdays etc.) But some countries advance their clock for 1 hour at certain times of the year. That means sometimes there will be 167 or 169 hours between two consecutive sundays, tuesdays or thursdays etc. Frontend code knows about this change because it uses local time zone to process date related things. So it won't match timestamps generated from backend on some countries if you just say list all sundays with time 00.00 That's why adding 7*24 hours instead of 1 week works. This is the best way I could explain. I hope it is clear. |
|
I think you misunderstood. I mean time when tests run can vary 24h between time zones and such issues are very common with unit tests that involve time often to the point that one has to "freeze" time during tests to get stable results. By using thursday, a timestamp can only shift by at most 24h via timezone offset, so it won't shift into the next or previous week. |
|
Let's merge this in 24h. I think it's good enough and the change to Monday can follow separately. |
|
@sahinakkaya thanks for you work and also for your persistance 👍. |
|
Thank you everyone who contributed by their comments, reviews, and suggestions. I learned a lot in this process. And hey, we have a lovely contributors page now! 🥳 🎉 |
* giteaofficial/main: (23 commits) Remove jQuery from SSH key form parser (go-gitea#29193) Refactor request function (go-gitea#29187) Docker Tag Information in Docs (go-gitea#29047) Fix gitea-action user avatar broken on edited menu (go-gitea#29190) Disable parallel Make execution (go-gitea#29186) Auto-update the system status in admin dashboard (go-gitea#29163) Avoid vue warning in dev mode (go-gitea#29188) Update JS and PY dependencies (go-gitea#29184) [skip ci] Updated translations via Crowdin Implement contributors graph (go-gitea#27882) Add support for action artifact serve direct (go-gitea#29120) Advertise WebAuthn support (go-gitea#29176) Tweak repo header (go-gitea#29134) Change webhook-type in create-view (go-gitea#29114) Remove jQuery from the comment task list (go-gitea#29170) Fix can not select team reviewers when reviewers is empty (go-gitea#29174) move sign in labels to be above inputs (go-gitea#28753) Refactor locale&string&template related code (go-gitea#29165) Extract linguist code to method (go-gitea#29168) bump to use go 1.22 (go-gitea#29119) ...
Continuation of #25439. Fixes #847
Before:
After:
Overview
This is the implementation of a requested feature: Contributors graph (#847)
It makes Activity page a multi-tab page and adds a new tab called Contributors. Contributors tab shows the contribution graphs over time since the repository existed. It also shows per user contribution graphs for top 100 contributors. Top 100 is calculated based on the selected contribution type (commits, additions or deletions).
Demo
(The demo is a bit old but still a good example to show off the main features)
Screen.Recording.2023-07-11.at.10.51.37.mov
Features:
Issues to consider before merging
For big repositories, time required to fetch the data increases dramatically. We might consider setting a cron job as @lunny mentioned which I have no idea how to do so.(this is fixed 🥳 we now have cache. Cron jobs would be even better but I think caching is good for now. Maybe we can add cron job in another PR)When contribution type is changed by user, whole page is re-rendered causing fetching the same data again which is unnecessary. This can be easily avoided by passing only the selected option to Vue. But I don't know how to do it so I need help here.(this is fixed 🥳)When contribution type is changed by user, we probably need to sort per user graphs based on the selected contribution type and not commits (which is default).When user zooms or pans the graphs, we need to update title of the page to show correct dates. Currently this is hard-coded. (From repository start date - To current date)When user zooms or pans the graphs, we probably need to update statistics on per user graphs (x commits, y additions, z deletions) to account only commits from the selected date range. Currently, they are calculated from start date to end date and sorted by total commits.iDontCareAboutErrors, _ := someFunc()while reviewing the code :D We probably need to handle them correctly so please point me in the right direction.