Fix wrong timestamps for files with the same name
What does this MR do and why?
This pr should fix showing wrong commit dates in files with same name
Screenshots or screen recordings
Before | After |
---|---|
![]() |
![]() |
How to set up and validate locally
Steps to reproduce
- Create the folder structure above:
- In the first commit, add the
alpha
folder and it's contents, and set the commit message to "alpha". - In the first commit, add the
beta
folder and it's contents, and set the commit message to "beta".
- In the first commit, add the
- Open the "beta" folder and observe that all the files in this folder have the "beta" commit message and timestamps.
- Using the breadcrumbs, navigate to the parent folder and then click into the "alpha" folder.
- At this point, the files in the "alpha" folder will show "beta" as the commit message along with the beta timestamps, with the exception of the "alpha" file.
- If you then refresh the page, the "alpha" files will then show the correct "alpha" commit message and timestamps.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #363436 (closed)
Merge request reports
Activity
changed milestone to %Backlog
requested review from @jerasmus
assigned to @nradina
Suggested Reviewers (beta)
The individuals below may be good candidates to participate in the review based on various factors.
You can use slash commands in comments to quickly assign
/assign_reviewer @user1
.Suggested Reviewers @ntepluhina
,@kushalpandya
,@iamphill
,@vitallium
,@jivanvl
If you do not believe these suggestions are useful, please apply the label Bad Suggested Reviewer. You can also provide feedback for this feature on this issue:
https://gitlab.com/gitlab-org/gitlab/-/issues/357923
.Automatically generated by Suggested Reviewers Bot - an experimental ML-based recommendation engine created by ~"group::applied ml".
Edited by GitLab Reviewer-Recommender Bot2 Warnings b830ad74: The commit subject must start with a capital letter. For more information, take a look at our Commit message guidelines. b830ad74: The commit subject and body must be separated by a blank line. For more information, take a look at our Commit message guidelines. If needed, you can retry the
danger-review
job that generated this comment.Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer frontend Stanislav Lashmanov ( @slashmanov
) (UTC+4)Illya Klymov ( @xanf
) (UTC+3)To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
Generated by
DangerBundle size analysis [beta]
This compares changes in bundle size for entry points between the commits 9d6ff8f9 and f0dc9c3d
Special assetsEntrypoint / Name Size before Size after Diff Diff in percent average 3.54 MB 3.54 MB - 0.0 % mainChunk 1.98 MB 1.98 MB - 0.0 %
Note: We do not have exact data for 9d6ff8f9. So we have used data from: 2e888a8a.
The target commit was too new, so we used the latest commit from master we have info on.
It might help to rerun thebundle-size-review
job
This might mean that you have a few false positives in this report. If something unrelated to your code changes is reported, you can check this comparison in order to see if they caused this change.Please look at the full report for more details
Read more about how this report works.
Generated by
DangerAllure report
allure-report-publisher
generated test report!review-qa-blocking:
test report for f0dc9c3dexpand test summary
+---------------------------------------------------------------------------+ | suites summary | +----------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +----------------------+--------+--------+---------+-------+-------+--------+ | Create | 23 | 0 | 2 | 23 | 25 | ❗ | | Verify | 12 | 0 | 1 | 12 | 13 | ❗ | | Plan | 48 | 0 | 1 | 48 | 49 | ❗ | | Manage | 37 | 0 | 2 | 38 | 39 | ❗ | | Version sanity check | 0 | 0 | 1 | 0 | 1 | ➖ | | Secure | 2 | 0 | 0 | 2 | 2 | ❗ | | Protect | 2 | 0 | 0 | 2 | 2 | ❗ | | Package | 0 | 0 | 1 | 0 | 1 | ➖ | | Configure | 0 | 0 | 1 | 0 | 1 | ➖ | +----------------------+--------+--------+---------+-------+-------+--------+ | Total | 124 | 0 | 9 | 125 | 133 | ❗ | +----------------------+--------+--------+---------+-------+-------+--------+
added 466 commits
-
424b3864...eaf9404f - 465 commits from branch
master
- efbd5d1b - 363436 fix wrong timestamps for files with the same name
-
424b3864...eaf9404f - 465 commits from branch
- Resolved by Nataliia Radina
@nradina thanks for this! The fix looks good, we'll just need to ensure we have sufficient test coverage.
Something like this should do the trick:
diff --git a/spec/frontend/repository/components/table/index_spec.js b/spec/frontend/repository/components/table/index_spec.js index ff0371b5c07..481d006798a 100644 --- a/spec/frontend/repository/components/table/index_spec.js +++ b/spec/frontend/repository/components/table/index_spec.js @@ -11,7 +11,7 @@ const MOCK_BLOBS = [ { id: '123abc', sha: '123abc', - flatPath: 'blob', + flatPath: 'main/blob.md', name: 'blob.md', type: 'blob', webPath: '/blob', @@ -19,7 +19,7 @@ const MOCK_BLOBS = [ { id: '124abc', sha: '124abc', - flatPath: 'blob2', + flatPath: 'main/blob2.md', name: 'blob2.md', type: 'blob', webUrl: 'http://test.com', @@ -27,7 +27,7 @@ const MOCK_BLOBS = [ { id: '125abc', sha: '125abc', - flatPath: 'blob3', + flatPath: 'main/blob3.md', name: 'blob3.md', type: 'blob', webUrl: 'http://test.com', @@ -38,6 +38,7 @@ const MOCK_BLOBS = [ const MOCK_COMMITS = [ { fileName: 'blob.md', + filePath: 'main/blob.md', type: 'blob', commit: { message: 'Updated blob.md', @@ -45,6 +46,7 @@ const MOCK_COMMITS = [ }, { fileName: 'blob2.md', + filePath: 'main/blob2.md', type: 'blob', commit: { message: 'Updated blob2.md', @@ -52,6 +54,7 @@ const MOCK_COMMITS = [ }, { fileName: 'blob3.md', + filePath: 'main/blob3.md', type: 'blob', commit: { message: 'Updated blob3.md',
Could you please take a look and assign back to me afterward?
removed review request for @jerasmus
@nradina it looks like this fix is working great, however, the commits aren't being found when viewing the project root:
I think I can see why. Upon checking the value of
commitEntry.filePath
in thegetCommit
method, I can see two//
is appended to the file name. I've narrowed it down to thenormalizeData
function where we determine thefilePath
. You could either try to fix it in thenormalizeData
function or you could try and use ourcleanLeadingSeparator
utility for cleaning the leading slashes:diff --git a/app/assets/javascripts/repository/components/table/index.vue b/app/assets/javascripts/repository/components/table/index.vue index dcf085c62c5..f22a20adf38 100644 --- a/app/assets/javascripts/repository/components/table/index.vue +++ b/app/assets/javascripts/repository/components/table/index.vue @@ -2,6 +2,7 @@ import { GlSkeletonLoader, GlButton } from '@gitlab/ui'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { sprintf, __ } from '~/locale'; +import { cleanLeadingSeparator } from '~/lib/utils/url_utility'; import getRefMixin from '../../mixins/get_ref'; import projectPathQuery from '../../queries/project_path.query.graphql'; import TableHeader from './header.vue'; @@ -109,7 +110,7 @@ export default { } return this.commits.find( - (commitEntry) => commitEntry.filePath === flatPath && commitEntry.type === type, + (commitEntry) => cleanLeadingSeparator(commitEntry.filePath) === flatPath && commitEntry.type === type, ); }, },
Edited by Jacques Erasmusadded 97 commits
-
5710692e...fa45c13b - 94 commits from branch
master
- 20ae0b50 - 363436 fix wrong timestamps for files with the same name
- 9eaca795 - Fix failing test for table
- 3b2b5475 - Add cleanLeadingSeparator function
Toggle commit list-
5710692e...fa45c13b - 94 commits from branch
added workflowin review label
added 25 commits
-
3b2b5475...ca750b6c - 22 commits from branch
master
- 3aab17ae - 363436 fix wrong timestamps for files with the same name
- cd55682b - Fix failing test for table
- 3041f389 - Add cleanLeadingSeparator function
Toggle commit list-
3b2b5475...ca750b6c - 22 commits from branch
added 1 commit
- ce8ff6d6 - Encode ref when making request for commit data
requested review from @iamphill
marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed
requested review from @vitallium
removed review request for @iamphill
changed milestone to %15.2
added 515 commits
-
ce8ff6d6...032c4661 - 511 commits from branch
master
- b830ad74 - 363436 fix wrong timestamps for files with the same name
- 7c618966 - Fix failing test for table
- 8b2189f9 - Add cleanLeadingSeparator function
- f0dc9c3d - Encode ref when making request for commit data
Toggle commit list-
ce8ff6d6...032c4661 - 511 commits from branch
@vitallium
, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.
For more info, please refer to the following links:
enabled an automatic merge when the pipeline for f470b997 succeeds
mentioned in commit 1e6aa651
removed workflowin review label
added workflowverification label
added workflowstaging-canary label and removed workflowverification label
added workflowcanary label and removed workflowstaging-canary label
added workflowstaging label and removed workflowcanary label
added workflowproduction label and removed workflowstaging label
mentioned in commit 87193541
mentioned in commit a2b84d4e
mentioned in commit 814a581b
mentioned in issue #363436 (closed)
added releasedcandidate label
added releasedpublished label and removed releasedcandidate label