apache
Fix link to removed volumes being shown in info card and list view
Description
The volume snapshots and the snapshot details page show a link to the snapshotted volume. However, if the volume has been removed, it is not possible to access its page, resulting in a 404 when a user clicks the link.
This PR adds a validation in the UI in order to avoid showing this link when the volume has been removed.
Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
Feature/Enhancement Scale or Bug Severity
Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [X] Minor
- [ ] Trivial
Screenshots (if appropriate):
When the volume exists, a link is shown.
If the volume has been removed, the link does not get shown.
How Has This Been Tested?
- I created a volume snapshot and verified that the volume snapshots and the snapshot details page showed a link to the volume;
- I removed the volume and verified that the link was not shown anymore.
Codecov Report
Attention: Patch coverage is 21.52918% with 390 lines in your changes missing coverage. Please review.
Project coverage is 15.42%. Comparing base (
53faf0f) to head (e558ca6). Report is 18 commits behind head on 4.19.
:exclamation: Current head e558ca6 differs from pull request most recent head 62e405d
Please upload reports for the commit 62e405d to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## 4.19 #8833 +/- ##
============================================
+ Coverage 14.97% 15.42% +0.45%
- Complexity 11049 11818 +769
============================================
Files 5390 5475 +85
Lines 470964 478624 +7660
Branches 59210 60820 +1610
============================================
+ Hits 70528 73834 +3306
- Misses 392599 396657 +4058
- Partials 7837 8133 +296
| Flag | Coverage Δ | |
|---|---|---|
| uitests | 4.21% <ø> (-0.07%) |
:arrow_down: |
| unittests | 16.18% <21.52%> (+0.49%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
@sureshanaparti a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
UI build: :heavy_check_mark: Live QA URL: https://qa.cloudstack.cloud/simulator/pr/8833 (QA-JID-309)
@winterhazel , I marked this for 4.20 now as it is targeted for main. Do you want this in 4.19?
cc @lucas-a-martins
Thanks for testing @lucas-a-martins. At first, I think the issue regarding invalid links in the list view should be tackled in another PR, but I'll investigate how it could be implemented to see if it makes sense to unify how the validation is performed with this one.
@winterhazel , I marked this for 4.20 now as it is targeted for main. Do you want this in 4.19?
@DaanHoogland sure, I'll target 4.19.
Hey @lucas-a-martins @DaanHoogland, sorry for the delay.
Looking back at this PR, I think it would be better to implement the link validation in both pages by including the volume's state in the main response and verifying it in the UI, as the approach here (calling another API to obtain a single volume's data) would become unfeasible when we need to validate links for a lot of resources (such as in the volume snapshots page). I suppose this would also be the ideal approach for validating most links to resources.
I will make some changes in this PR to adjust this, and to include the validation in the volume snapshots page as well.
@lucas-a-martins @DaanHoogland, I changed how the link validation is performed, and included it in the list view as well. Could you guys take another look at this PR?
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10444
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
[SF] Trillian test result (tid-10950) Environment: kvm-alma9 (x2), Advanced Networking with Mgmt server a9 Total time taken: 46197 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8833-t10950-kvm-alma9.zip Smoke tests completed. 132 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below:
| Test | Result | Time (s) | Test File |
|---|
Merging based on approvals, CI result and manual tests by @lucas-a-martins
