Fix history displays irrelevant changes #922
Conversation
thachlp
requested review from
ikhoon,
jrhee17,
minwoox and
trustin
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #922 +/- ##
============================================
+ Coverage 66.78% 67.52% +0.74%
- Complexity 3513 3567 +54
============================================
Files 370 370
Lines 14476 14477 +1
Branches 1553 1553
============================================
+ Hits 9668 9776 +108
+ Misses 3929 3821 -108
- Partials 879 880 +1 ☔ View full report in Codecov by Sentry. |
|
Could you also check if |
Ok, let me check |
Sure, let me try. |
| final AggregatedHttpResponse userInfo = client.get("/api/v0/users/me").aggregate().join(); | ||
| final JsonNode jsonNode = Jackson.readTree(userInfo.contentUtf8()); | ||
| assertThat(jsonNode.get("login").asText()).isEqualTo("[email protected]"); | ||
| } |
There was a problem hiding this comment.
@minwoox here, I try to add tests for the class RepositoryServiceTest, but it always returns 404 for /api/v0 (I try users/me to make sure I am correct ^^)
Do you know how to start the /api/v0 dogma for the test?
There was a problem hiding this comment.
It's activated only when the webAppEnabled is true.
By the way, could you tell me why you need to retrieve the user information?
There was a problem hiding this comment.
Thanks @minwoox, I can call /api/v0 now.
I just do the simplest test with api/v0 to explain to you my problem 🤩
| @@ -177,10 +177,11 @@ public CompletionStage<List<CommitDto>> getHistory(@Param String projectName, | |||
| @Param String path, | |||
| @Param @Default("-1") String from, | |||
| @Param @Default("1") String to) { | |||
| final String pathPattern = "/".equals(path) ? "/**" : path + "/**"; | |||
There was a problem hiding this comment.
The other APIs we have (e.g. watch*, diff*) don't add a slash at the end.
For consistency, I think it would be better that all APIs that accept a pathPattern have the same behavior.
Also, I think it is perfectly valid to watch /a** depending on the use-case.
Question) What do you think of fixing this issue from the client side instead?
There was a problem hiding this comment.
Sorry about the late reply. Yeah, I also thought we should fix the UI (which is the client) to call the path correctly when I first created the issue.
We should let users retrieve whatever history they want so I think we should touch it on the server-side.
There was a problem hiding this comment.
Sound good @minwoox, could you provide me the screenshot to explain expected from client? I am happy to finish this 🚀
Motivate:
The history of a directory includes changes in directories share the name. For instance: the history of
/foodisplays changes for/foo2,/foo3,/foobar...Modifications:
Add a slash
(/)before**in the path(path + "/**"). The patternfoo/**matches/foo/anything,/foo/subdir/file, but it does not match anything that isn't a subpath of/foo.Result
Fixes #902