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

Clean up viewer logic #2336

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Clean up viewer logic #2336

merged 1 commit into from
Sep 19, 2024

Conversation

laritakr
Copy link
Collaborator

@laritakr laritakr commented Sep 19, 2024

Refs scientist-softserv/adventist_knapsack#760

There are no expected behavioral changes due to these changes beyond the Bulkrax update... it is simply some cleanup, documentation, and behind the scenes work needed to prepare for some cleanup & behavior changes in Adventist's knapsack repo.

  • Updates Bulkrax
  • Add documentation to work_show_presenter
  • Update representative_media bootstrap classes
  • Update _relationships partial to coordinate with current Hyrax
  • Update some obsolete code in image_show/show.html.erb

- Updates Bulkrax
- Add documentation to work_show_presenter
- Update representative_media bootstrap classes
- Update _relationships partial to coordinate with current Hyrax
- Update some obsolete code in image_show/show.html.erb
@laritakr laritakr added the ignore-for-release ignore this for release notes label Sep 19, 2024
Copy link

Test Results

    3 files  ±0      3 suites  ±0   18m 15s ⏱️ -18s
2 036 tests ±0  1 986 ✅ ±0  50 💤 ±0  0 ❌ ±0 
2 063 runs  ±0  2 011 ✅ ±0  52 💤 ±0  0 ❌ ±0 

Results for commit 7d7b434. ± Comparison against base commit dac6f86.

This pull request removes 42 and adds 42 tests. Note that renamed tests count towards both.
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to destroy 1f36c78e-eb2e-4e2e-a5d1-4b19a713c521
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to edit 95bede3c-f526-4730-84e3-46d9eb76fb6f
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to read 1cf10b13-c622-4e49-8c11-c995e685ab00
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to update ff3d5444-a666-4864-aa10-34f607fcd8a3
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to destroy 844e061a-5160-4cdc-800b-414b0ce9459d
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to edit 52e87a3d-f509-46af-8c2e-5e90f87175d8
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to read 95394656-ef39-47a3-8520-65860c572555
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to update 2d176015-0f1e-4291-93c2-3c0b79277195
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to destroy df1d1683-efa8-4992-8f56-144821d61a0d
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to edit b60de19b-77df-4a26-ad15-556e25306dcf
…
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to destroy 36b22e1f-4eff-4764-9a9b-03c90e1ad7fa
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to edit 332cbbe3-0673-4844-8370-d4417b4e09c8
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to read 2b75a80b-3565-4448-8d09-42268b6b64c3
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor Etd permissions is expected not to be able to update 88e34a79-6240-473f-a8f9-066d3d902e63
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to destroy 64f44427-43d2-4187-a7b3-60b1099c88e4
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to edit fd7ae5c9-ef62-4e5d-8d2d-3fe9efb6135d
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to read d709e8b0-61a2-4d2d-8e6c-3c73ee96b953
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor FileSet permissions is expected not to be able to update 6bd0dbc4-b384-46be-82b3-58103c9e2e93
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to destroy 74a152e0-e10d-4241-8c19-dac811e2d10b
spec.abilities.work_ability_spec ‑ Hyrax::Ability::WorkAbility when work depositor GenericWork permissions is expected not to be able to edit 59703e7c-8d69-4787-b943-15a68cecf6f0
…

@laritakr laritakr merged commit c09966b into main Sep 19, 2024
8 of 9 checks passed
@laritakr laritakr deleted the media-viewer-logic branch September 19, 2024 21:23
laritakr added a commit to scientist-softserv/adventist_knapsack that referenced this pull request Sep 19, 2024
Overrides logic related to media viewers to prioritize UV in all
cases, and fallback to PDFjs as much as possible, regardless of
feature flipper setting.

Refs
- samvera/hyku#2336
- #817

- Clean up views related to media viewers
- Better documentation of overrides
- Remove duplicated file set indexer decorator
ShanaLMoore added a commit to scientist-softserv/adventist_knapsack that referenced this pull request Sep 20, 2024
# Story

Overrides logic related to media viewers to prioritize UV in all
cases, and fallback to PDFjs as much as possible, regardless of
feature flipper setting.

Refs
- samvera/hyku#2336
- #760

Other changes:
- Clean up views related to media viewers
- Better documentation of overrides
- Remove duplicated file set indexer decorator
- Update submodule to bring in Hyku changes & Bulkrax update

# Expected Behavior Before Changes

Media viewers worked based on flipper flags. Behavior in error
situations or during upload process was unexpected.
Based on flipper setting and split status, we see:

Default viewer PDF.js, no split >> shows pdfs viewer
Default viewer: UV, no split >> uses representative_media partial and UV
- black box because no split
Default viewer PDF.js, split into child works, >> no viewer, shows
default thumbnail
Default viewer: UV, split >> uses UV

# Expected Behavior After Changes

**Flipper set to PDF.js**
- [ ] new works do not split into child work pages
- [ ] uses PDFjs viewer if work does not have child work pages
- [ ] uses UV if work was previously split into child work pages

**Flipper set to UV**
- [ ] new works will split into child work pages
- [ ] uses PDFjs viewer until split is complete
- [ ] uses UV once child work split is complete

# Screenshots / Video

<details>
<summary>Flipper set to PDF.js</summary>

![Screenshot 2024-09-19 at 5 53
56 PM](https://github.com/user-attachments/assets/4e63e6c0-0e13-4cec-a288-7c32914e9347)
![Screenshot 2024-09-19 at 5 53
48 PM](https://github.com/user-attachments/assets/aee35512-f3e1-4ebd-9d80-2f621e0b3bb7)
![Screenshot 2024-09-19 at 5 53
25 PM](https://github.com/user-attachments/assets/f1907978-f581-49f0-bf4d-7117dd1ede21)

</details>

<details>
<summary>Flipper set to UV</summary>

![Screenshot 2024-09-19 at 5 52
38 PM](https://github.com/user-attachments/assets/7efc809b-e6db-414e-ba10-6561a082c3ee)
![Screenshot 2024-09-19 at 5 52
14 PM](https://github.com/user-attachments/assets/75cf45bb-ba15-47d1-935e-0426978524a2)
![Screenshot 2024-09-19 at 5 52
10 PM](https://github.com/user-attachments/assets/99aac40a-9334-41a8-9c82-24821a252f1c)

</details>

# Notes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-for-release ignore this for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants