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

Fix all events link #7059

Merged
merged 2 commits into from
Oct 3, 2022
Merged

Fix all events link #7059

merged 2 commits into from
Oct 3, 2022

Conversation

nwmac
Copy link
Member

@nwmac nwmac commented Sep 30, 2022

Fixes #5816

Addresses 2 problems when clicking on the 'Show all events' link on the cluster dashboard page.

  • Error parsing community links when it is set to {} - which can happen
  • Error on the link used for the event resource page

@github-actions github-actions bot added this to the v2.7.0 milestone Sep 30, 2022
Comment on lines 66 to 79
if (this.uiCustomLinks?.value) {
try {
const customLinks = JSON.parse(this.uiCustomLinks.value);

if (Array.isArray(customLinks)) {
return JSON.parse(this.uiCustomLinks.value).reduce((prev, curr) => {
prev[curr.key] = curr.value;

return prev;
}, {});
}
} catch (e) {
console.error('Could not parse custom links setting', e); // eslint-disable-line no-console
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (this.uiCustomLinks?.value) {
try {
const customLinks = JSON.parse(this.uiCustomLinks.value);
if (Array.isArray(customLinks)) {
return JSON.parse(this.uiCustomLinks.value).reduce((prev, curr) => {
prev[curr.key] = curr.value;
return prev;
}, {});
}
} catch (e) {
console.error('Could not parse custom links setting', e); // eslint-disable-line no-console
}
if (!!this.uiCustomLinks?.value) {
const val = JSON.parse(this.uiCustomLinks.value);
// User could have no links.
const customLinks = Array.isArray(val) ? val : Object.keys({});
return customLinks.reduce((prev, curr) => {
prev[curr.key] = curr.value;
return prev;
}, {});
}

Copy link
Member

Choose a reason for hiding this comment

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

Unless I'm missing something here I think .reduce is fine on an empty array if there's an initial value set? Other than my uncertainty around this the PR looks good.

Comment on lines 66 to 79
if (this.uiCustomLinks?.value) {
try {
const customLinks = JSON.parse(this.uiCustomLinks.value);

if (Array.isArray(customLinks)) {
return JSON.parse(this.uiCustomLinks.value).reduce((prev, curr) => {
prev[curr.key] = curr.value;

return prev;
}, {});
}
} catch (e) {
console.error('Could not parse custom links setting', e); // eslint-disable-line no-console
}
Copy link
Member

Choose a reason for hiding this comment

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

Unless I'm missing something here I think .reduce is fine on an empty array if there's an initial value set? Other than my uncertainty around this the PR looks good.

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@1c6b5ac). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #7059   +/-   ##
=========================================
  Coverage          ?   36.30%           
=========================================
  Files             ?      967           
  Lines             ?    17222           
  Branches          ?     4462           
=========================================
  Hits              ?     6253           
  Misses            ?    10969           
  Partials          ?        0           
Flag Coverage Δ
e2e 46.76% <0.00%> (?)
merged 36.30% <0.00%> (?)
unit 5.29% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@aalves08 aalves08 left a comment

Choose a reason for hiding this comment

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

LGTM

@nwmac nwmac merged commit 8d0701d into rancher:master Oct 3, 2022
@nwmac nwmac deleted the fix-events branch October 3, 2022 09:05
bisht-richa pushed a commit to bisht-richa/dashboard that referenced this pull request Oct 5, 2022
* Fix all events link

* Minor update
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.

UI 2.6.x 'events' view is confusing and 'hidden' by default
5 participants