Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: show issueExtraDetails data on treeDetails #909

Merged
merged 6 commits into from
Feb 13, 2025

Conversation

MarceloRobert
Copy link
Collaborator

@MarceloRobert MarceloRobert commented Feb 11, 2025

  • Adds functions to fetch extra issue details
  • Use these functions in treeDetailsLazyLoaded (requires summaryData for the request)
  • Changes Issue List to occupy 2 columns and changes how it's truncated
  • Adds text for first_seen and badges for the tags

How to test

Go to any treeDetails that have issues; all of them should have at least the first_seen information, the tags are optional and depend on each issue in particular.

Examples are sashal-next/linus-next, stable/linux-6.6.y and android/android12-5.10-lts.

You can check every tree that the issues are present by looking in the network tab of dev tools and see the response for the /issue/extras/ request.

Reference (from stabe/linux-6.6.y):
image

Closes #870

@MarceloRobert MarceloRobert force-pushed the feat/more-issue-info-FE branch 2 times, most recently from f53d62c to 7248507 Compare February 11, 2025 20:04
@MarceloRobert MarceloRobert self-assigned this Feb 11, 2025
dashboard/src/types/issueExtras.ts Outdated Show resolved Hide resolved
dashboard/src/types/issueExtras.ts Outdated Show resolved Hide resolved
dashboard/tailwind.config.js Outdated Show resolved Hide resolved
Comment on lines 4 to 8
enum PossibleIssueTags {
Mainline = 'mainline',
Stable = 'stable',
LinuxNext = 'linux-next',
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://dev.to/ivanzm123/dont-use-enums-in-typescript-they-are-very-dangerous-57bh

Suggested change
enum PossibleIssueTags {
Mainline = 'mainline',
Stable = 'stable',
LinuxNext = 'linux-next',
}
const PossibleIssueTags = {
mainline: 'mainline',
stable: 'stable',
linuxNext : 'linux-next',
} as const

@@ -150,6 +140,19 @@ const BuildTab = ({ treeDetailsLazyLoaded }: BuildTab): JSX.Element => {
diffFilter={diffFilter}
/>
</div>
<MemoizedIssuesList
title={<FormattedMessage id="global.issues" />}
issues={treeDetailsData.buildsIssues || []}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
issues={treeDetailsData.buildsIssues || []}
issues={treeDetailsData.buildsIssues ?? []}

Comment on lines +26 to +31
issuesExtras: {
data?: IssueExtraDetailsResponse;
isLoading: boolean;
status: QuerySelectorStatus;
error: UseQueryResult['error'];
};
Copy link
Collaborator

@WilsonNet WilsonNet Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you can add a generic type

  type TypeName = <T> {
    data?: T;
    isLoading: boolean;
    status: QuerySelectorStatus;
    error: UseQueryResult['error'];
  };

type IssueExtrar = TypeName<IssueExtraDetailsResponse>

}: ITabsIssues): IssueKeyList => {
const result: IssueKeyList = [];

for (const issue of buildIssues || []) {
Copy link
Collaborator

@WilsonNet WilsonNet Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  buildIssues.forEach...

for (const issue of buildIssues || []) {
result.push([issue.id, issue.version]);
}
for (const issue of bootIssues || []) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto foreach

for (const issue of bootIssues || []) {
result.push([issue.id, issue.version]);
}
for (const issue of testIssues || []) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto foreach

@MarceloRobert MarceloRobert force-pushed the feat/more-issue-info-FE branch from 7248507 to 8a2d715 Compare February 12, 2025 17:36
Copy link
Collaborator

@WilsonNet WilsonNet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

Copy link
Contributor

@murilx murilx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! I'd only hope the component was a little bit more "smart" when deciding when to start hiding the issue comment but it's working nice as is

@MarceloRobert MarceloRobert force-pushed the feat/more-issue-info-FE branch from 8a2d715 to 013e746 Compare February 13, 2025 14:12
@MarceloRobert MarceloRobert force-pushed the feat/more-issue-info-FE branch from 013e746 to ae109d3 Compare February 13, 2025 16:21
@MarceloRobert MarceloRobert merged commit 381a480 into main Feb 13, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IssuesList and IssueDetails frontend changes
3 participants