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: [M3-9235] - Longview Detail id param not found locally #11599

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

abailly-akamai
Copy link
Contributor

@abailly-akamai abailly-akamai commented Feb 3, 2025

Description 📝

when going to a longview client detail (and only locally) we get a

Cannot read properties of undefined (reading 'id') 

error.

Interestingly, Production is unaffected and this was not an error when rerouting the features in #11490

Changes 🔄

  • Ensure params.id is passed to both the base & params EnhancedLongviewDetail props to satisfy redux

Target release date 🗓️

2/11/2025

Preview 📷

Before After
Screenshot 2025-02-03 at 15 13 46 Screenshot 2025-02-03 at 15 14 07

How to test 🧪

Prerequisites

  • Have a working Longview client. ex
  • Create a new Linode (use Debian)
  • Create a new Longview client
  • Copy the installation curl command
  • SSH into your Linode and paste the installation command

Note: You can start the longview service with systemctl: sudo systemctl start longview if nothing is happening after a while

👉 following https://www.linode.com/docs/products/tools/longview/guides/troubleshooting/

Verification steps

  • Verify the Longview Detail client (once available) can be navigated to
  • run yarn && yarn build && yarn start:manager:ci to serve a build and ensure no regression
  • All existing tests should pass
Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support

  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@abailly-akamai abailly-akamai self-assigned this Feb 3, 2025
@abailly-akamai abailly-akamai marked this pull request as ready for review February 3, 2025 20:37
@abailly-akamai abailly-akamai requested a review from a team as a code owner February 3, 2025 20:37
@abailly-akamai abailly-akamai requested review from bnussman-akamai and hana-akamai and removed request for a team February 3, 2025 20:37
match: {
params: {
id,
},
},
params: {
id,
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If anyone has any clue why this was working without this when implemented previously (and why this isn't an issue in production), please let me know. Considering this is working fine and that Longview will be deprecated in the not so long term, I did not do a full investigation.

Copy link

github-actions bot commented Feb 3, 2025

Coverage Report:
Base Coverage: 79.28%
Current Coverage: 79.28%

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 497 passing tests on test run #2 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing497 Passing2 Skipped98m 21s

@hana-akamai
Copy link
Contributor

Confirmed the Longview Detail client can be navigated to locally now and also via the PR preview link. Not sure why this broke 🤔

@hana-akamai hana-akamai added the Add'tl Approval Needed Waiting on another approval! label Feb 4, 2025
@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Feb 4, 2025
@abailly-akamai abailly-akamai merged commit 02ee508 into linode:develop Feb 4, 2025
23 checks passed
Copy link

cypress bot commented Feb 4, 2025

Cloud Manager E2E    Run #7171

Run Properties:  status check passed Passed #7171  •  git commit 02ee5089d1: fix: [M3-9235] - Longview Detail id param not found locally (#11599)
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #7171
Run duration 29m 37s
Commit git commit 02ee5089d1: fix: [M3-9235] - Longview Detail id param not found locally (#11599)
Committer Alban Bailly
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 3
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 499
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Routing Refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants