-
Notifications
You must be signed in to change notification settings - Fork 1
[PER-10348] Fix public gallery link for records #820
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #820 +/- ##
==========================================
- Coverage 47.06% 47.06% -0.01%
==========================================
Files 372 372
Lines 11492 11495 +3
Branches 1907 1909 +2
==========================================
+ Hits 5409 5410 +1
Misses 5897 5897
- Partials 186 188 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d96be6a to
1928e24
Compare
| value.parentFolderArchiveNumber && | ||
| value.parentFolder_linkId && | ||
| rootArchive !== value.parentFolderArchiveNumber | ||
| ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens from a UX PoV if this else block's logical conditions are not met? Do we need to cover that case somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parent folder is not added to the root, so the user will land on the public folder. Maybe this should be done indeed better, but I do think it is out of the scope of this bug.
cecilia-donnelly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed with @slifty's point about the backend response intentionally being smaller. I'm impressed you were able to work around this, @aasandei-vsp! I do wonder if the extra parameters mean that old links would now be broken. Do you know? I don't think I have one handy unfortunately.
|
@cecilia-donnelly @slifty Sorry, I only added this on the story, forgot to put it on the PR: I have managed to find a fix for the issue, however, it is not ideal. The right way would be to get the ParentFolderVOs property from the BE, when we call v2/record. With the current fix, links are generated and they work. But there is an exception!
!!Both versions of the link work ok. TL:DR The old links should work too! |
cecilia-donnelly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works! Thanks @aasandei-vsp. Agreed with @slifty's comments about opening an issue and pointing to it from the comment.
1928e24 to
c5ab1d6
Compare
omnignorant
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to be working as expected now!
The public gallery links were not generating correctly, because the endpoint for getting the record has changed and the property used to create it, ParentFolderVOs, is not present anymore. In order to create the record links, parent properties present on the record object can be used. One exception is, the link for records that are in the root folder will have 2 extra parameters, but they will work. Issue: PER-10348 Get public gallery link not working for individual records
c5ab1d6 to
c8e7950
Compare
The public gallery links were not generating correctly, because the endpoint for getting the record has changed and the property used to create it, ParentFolderVOs, is not present anymore. Ideally we would fix the backend, but for now this will do the job. One exception is, the link for records that are in the root folder will have 2 extra parameters, but they will work.
Check the issue bellow for more details on how this should be tackled on BE.
Issue: PER-10348 Get public gallery link not working for individual records
STEPS TO TEST:
Root record
EXPECTED: A modal with a generated link will appear
EXPECTED:
Folder
EXPECTED: A modal with a generated link will appear
EXPECTED:
Record in folder
EXPECTED: A modal with a generated link will appear
EXPECTED: