fix the check run filter by id #18650
Conversation
|
Hey @saurabh-git-dev, thanks so much for taking the time to contribute to Desktop! I believe this change makes sense but I'd really like for @tidy-dev (the original author of this) to take a look as well. My concern is centered around the fact that we named this function getLatestCheckRunsByName so I want to make sure that id is a suitable replacement. If it is then we should update the name of the function as well. |
|
I did some digging to see if I could find reasoning for using the name versus the id. (I couldn't remember this method) @niik you are the original author of the function for the pr status work. I only added filter based on if it was a push or a pull. 1eed3bc#diff-e3f9f5058c055c6e578c6019115cad54a10a80455eeeb4c4afca17df1b86150cL352 Looking at the docs, I cannot see a reason not to use id here instead of name. |
There was a problem hiding this comment.
@saurabh-git-dev Could you please update the function name to getLatestCheckRunsById?
|
I actually had the same thought as to why the "id" was not used. In the shared picture there are 2 jobs with the same name. And also the same workflow name. That's why appear in a 'Test' group. I looked at the grouping method. But it is used in many places. So did not change that. desktop/app/src/lib/ci-checks/ci-checks.ts Line 598 in b80f540 Although there is not a big impact with the grouping issue. |
method name changed and formatted the code.
Closes #18648
Description
This issue is due to filtering by check-run "name" instead of filtering by "id". It is replaced with "id" as we get check-run "id" in the API response(Attached API ref.).
If we change the name with id. We can see all the jobs with the same name.
API Ref:
https://docs.github.com/en/rest/checks/runs?apiVersion=2022-11-28#list-check-runs-for-a-git-reference
Screenshots