Skip to content

Conversation

@bwatson78
Copy link
Contributor

Fixes

Fixes #6874

Summary

Cleans ID array attributes before persisting with pair-tree Valkyrie Fedora.

Guidance for testing, such as acceptance criteria or new user interface behaviors:

  1. Run a local build of Sirenia using Docker
  2. Log in and navigate to Dashboard -> Works
  3. Click on the title of a work that you've already deposited (or deposit a new one if you don't have any works)
  4. Click Edit
  5. Select a different visibility setting (I went from Public to Institution) and click Save Changes
  6. A message saying Failed save on # undefined method split' for nil:NilClass should not appear, replaced with a success message.

Type of change (for release notes)

  • notes-bugfix Bug Fixes
  • notes-valkyrie Valkyrie Progress

Changes proposed in this pull request:

  • app/models/hyrax/change_set.rb: creates a method that cleans multi-valued id attributes if they contain empty strings.
  • lib/hyrax/transactions/steps/save.rb: calls the new method before syncing, where instances of Valkyrie:ID with empty ID values could be created.
  • spec/models/hyrax/change_set_spec.rb: tests the method created.

@samvera/hyrax-code-reviewers

@bwatson78 bwatson78 added bug notes-valkyrie Release Notes: Valkyrie specific notes-bugfix Release Notes: Fixed a bug labels Aug 9, 2024
@bwatson78 bwatson78 self-assigned this Aug 9, 2024
@dlpierce
Copy link
Member

dlpierce commented Aug 9, 2024

This seems to address the symptom of a bigger problem, likely where an unsaved object with a nil id is associated with this parent object via an *_id* field, and to_s is called on that nil for compatibility reasons (because it's presumed to be a Valkyrie::ID) and then the resulting empty string gets persisted.

@github-actions
Copy link

github-actions bot commented Aug 9, 2024

Test Results

    17 files  ±0      17 suites  ±0   2h 19m 29s ⏱️ + 4m 54s
 6 706 tests +2   6 409 ✅ +3  297 💤 ±0  0 ❌  - 1 
13 180 runs  +2  12 785 ✅ +3  395 💤 ±0  0 ❌  - 1 

Results for commit ff1a22c. ± Comparison against base commit e4f8a06.

This pull request removes 273 and adds 275 tests. Note that renamed tests count towards both.
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007f359f5a4ed0>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007fc613836540>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007f359e2bc958>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007fc61319d5d0>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy AdminSet: d3ad5e5a-b631-4647-9ba0-62b18509d0a4
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy Hyrax::AdministrativeSet: af2a1a03-44c2-4752-ba90-1d24e7d1f138
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit AdminSet: b5ed8b80-843a-4590-83eb-58f11dc5ae52
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit Hyrax::AdministrativeSet: bc697243-cf0c-4b13-953f-947943070070
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update AdminSet: 8fb7f762-e274-495f-b9c3-577bb717663b
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update Hyrax::AdministrativeSet: 9781c893-9201-4e30-b352-01e08dee570d
…
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007f58b6590798>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007fbf1fbc99c8>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007f58b673e248>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007fbf1fe8fb58>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy AdminSet: 5a14df86-5d7a-49f3-8b60-c7923484688c
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy Hyrax::AdministrativeSet: 18d5e35a-5074-43d0-aef1-b1b4f668be9a
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit AdminSet: 1b0aede5-32cf-4935-b6ae-38feca0e5442
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit Hyrax::AdministrativeSet: 2554afdf-a808-453d-9c77-200136ee0d18
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update AdminSet: 549f871c-f7b2-471e-94bc-85350dd68d5c
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update Hyrax::AdministrativeSet: 219bd406-93f7-41e9-b273-a8232d74116d
…

@bwatson78 bwatson78 marked this pull request as draft August 14, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug notes-bugfix Release Notes: Fixed a bug notes-valkyrie Release Notes: Valkyrie specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Updating a work visibility in sirenia generates Failed save on # undefined method `split' for nil:NilClass message in sirenia

3 participants