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

Empty collections displayed when clicking "View collections of this type" after trying deleting non-empty collection type #5522

Closed
MPLSFedResearchTZ opened this issue Mar 11, 2022 · 19 comments · May be fixed by #6176

Comments

@MPLSFedResearchTZ
Copy link

MPLSFedResearchTZ commented Mar 11, 2022

Descriptive summary

When clicking the button "View collections of this type" it should display all visible collections of this collection type.

image

Steps to reproduce the behavior

  1. Go to Settings-> Collection types to get a list of collections types.
  2. Deleting a collection type that contains some collections of this type.
  3. The pop-up box shows up which contains two buttons, "Cancel" and "View collections of this type"
  4. Click the "View collections of this type"
  5. The user is supposed to see all the collections of this type but it shows an empty list.
@eliotjordan eliotjordan self-assigned this Mar 17, 2022
@eliotjordan
Copy link
Contributor

I'm unable to replicate this ticket. @elrayle Is it possible some of the collection work that's already been done has fixed this?

@MPLSFedResearchTZ
Copy link
Author

MPLSFedResearchTZ commented Mar 18, 2022

The issue is still there. I reproduced it by deleting this "test collection type" collection type I created. Maybe there is something different in this collection type?

image

@eliotjordan
Copy link
Contributor

@MPLSFedResearchTZ Couple of questions:

  1. Are you testing this using the Dassie or EngineCart test apps?
  2. Have you pulled in the latest commits on the main branch?
  3. If so, try creating a new collection type and a new collection, and then test the delete.

@MPLSFedResearchTZ
Copy link
Author

I tested this on nurax-pg. On nurax-pg, I tried to create a new collection type and the problem was still there.

@eliotjordan
Copy link
Contributor

eliotjordan commented Mar 18, 2022

@MPLSFedResearchTZ nurax-pg is ~55 commits behind Hyrax main. I'll see about getting that updated and deployed.
bba8958...main

@elrayle
Copy link
Contributor

elrayle commented Apr 4, 2022

I updated to latest Hyrax and this still fails on nurax-pg. The problem is the filtering. It filters by Collection Type > nesting only && Collection Type > Collection. The second filter is actually the has_model_ssim which is Hyrax::PcdmCollection on nurax-pg. I don't see the need for the second filter. Since the results are displayed on Dashboard -> Collections which already filters to collections, this seems redundant.

@jlhardes
Copy link
Contributor

Remaining work listed in #3820 - Replace calls to ActiveFedora::Base.where (removes 7 refs to ActiveFedora) might be needed for this issue

@jessicahilt
Copy link

jessicahilt commented Aug 7, 2023

Let's try to reproduce this problem first. If reproducible, cross-check against in #3820 .

@jessicahilt jessicahilt self-assigned this Aug 21, 2023
@mcritchlow
Copy link
Contributor

mcritchlow commented Aug 23, 2023

I'm unable to reproduce this as described with nurax on the pg branch (testing with @OkayKris ). Essentially, I never get the modal telling me to delete the associated collections.

When clicking on Delete for the collection type I created (after creating/associating a few collections with it) I get:

image

And then

image

However, when I then navigate to Collections in the UI, I get the following error (which makes sense because I never got the modal telling me to delete the collections associated with the collection type):

image

@mcritchlow
Copy link
Contributor

mcritchlow commented Aug 25, 2023

Here's a follow up after (finally) getting a koppie environment up and running.

what i'm finding is that the collection type #collections method is returning [] because despite use_valkyrie being true the created collections are not actually being persisted in PG/valkyrie, they are instead somehow persisted in AF. So as a result, the modal to block deleting the collection type is never triggered.

https://github.com/samvera/hyrax/blob/valkyrie-collectiontype-deletemodal-fix/app/models/hyrax/collection_type.rb#L117-L117 is returning nothing

Whereas if I run the next AF query https://github.com/samvera/hyrax/blob/valkyrie-collectiontype-deletemodal-fix/app/models/hyrax/collection_type.rb#L118-L118 I will actually retrieve the 2 collections that I created and associated with the collection type I'm trying to delete.

There are quite a few different dev/test environments in play here, so I'm admittedly not sure what I should expect to be working in koppie. But:

[7] pry(#<Hyrax::CollectionType>)> ActiveFedora::Base.where(Hyrax.config.collection_type_index_field.to_sym => to_global_id.to_s).count
=> 2

@no-reply if you have any tips i'd be interested. I didn't think koppie was running active fedora at all.

@no-reply
Copy link
Contributor

@mcritchlow ActiveFedora::Base.where(Hyrax.config.collection_type_index_field.to_sym => to_global_id.to_s).count doesn't hit FCRepo, just solr. if the solr index is configured correctly, i'd expect that code to work.

it seems like this comment #5522 (comment) correctly identifies the issue. the search builder used for the My::CollectionsController#index action is filtering out all collections, because it limits has_model_ssim to Collection which isn't a model name that gets indexed in Koppie.

no-reply pushed a commit that referenced this issue Aug 30, 2023
the issue in #5522 is that search filters for collections aren't working
correctly on `.koppie` (when collections have `model_name` other than
`Collection`).

the relevant controller specs were completely stubbed on all the related
behavior. this unstubs them so there's a narrow test that fails related to the
issue.
no-reply pushed a commit that referenced this issue Aug 31, 2023
the issue in #5522 is that search filters for collections aren't working
correctly on `.koppie` (when collections have `model_name` other than
`Collection`).

the relevant controller specs were completely stubbed on all the related
behavior. this unstubs them so there's a narrow test that fails related to the
issue.
no-reply pushed a commit that referenced this issue Aug 31, 2023
the issue in #5522 is that search filters for collections aren't working
correctly on `.koppie` (when collections have `model_name` other than
`Collection`).

the relevant controller specs were completely stubbed on all the related
behavior. this unstubs them so there's a narrow test that fails related to the
issue.
mcritchlow pushed a commit that referenced this issue Aug 31, 2023
the issue in #5522 is that search filters for collections aren't working
correctly on `.koppie` (when collections have `model_name` other than
`Collection`).

the relevant controller specs were completely stubbed on all the related
behavior. this unstubs them so there's a narrow test that fails related to the
issue.
@OkayKris
Copy link

@no-reply still unable to replicate the original issue, the modal notifying the user that there are multiple collections in a collection type is being bypassed/ignored as referenced by this 5522 comment

@no-reply
Copy link
Contributor

@OkayKris i'm confused. i thought that this issue was fixed by the removal of RoleManager?

@OkayKris
Copy link

@no-reply i assumed so as well, but i guess it's something completely different. my and @mcritchlow 's guesses were there is something going on with the relationship of collections and collection types or something is wrong with assigning a collection to a collection type

@no-reply
Copy link
Contributor

@OkayKris that seems like a leap to me, and i'm wondering if we could start at trying to get a failing test.

it may also help to say where you're observing the behavior you describe (on what branch/commit, in what environment). i wasn't seeing it yesterday on the now merged work.

@OkayKris
Copy link

@no-reply sure working on latest commit on main in koppie with the fix from #6268

i feel that ./spec/features/collection_type_spec.rb:369 should fail because of expect(page).to have_content(not_empty_collection_type.title) but this passes with Shaun's fix, however when trying to replicate with Matt's work in dev, it fails. we:

  1. create a collection type
  2. create multiple collections of that collection type
  3. optional, create works that go into collections
  4. delete collection type
  5. expect You cannot delete this collection type because one or more collections of this type have already been created.

but we don't get that modal, instead we're allowed to delete the collection type right away.

aside from this test, i assume we're expecting this to execute https://github.com/samvera/hyrax/blob/main/app/models/hyrax/collection_type.rb#L156-L160

to give us a deleteDenyModal flag (lack of a better term here) such that the <!-- Modal window: Can't delete the collection type --> pops up.

instead, we're taken straight to deleteModal where <!-- Modal window: Delete collection type confirmation --> if that makes sense?

@no-reply
Copy link
Contributor

@OkayKris i'm still confused. is the behavior you're experiencing that the feature test fails or that, using some other setup, you're able to create a circumstance where it fails?

it dosen't fail for me, and if the issue is the latter case, i think some more information is needed. what's different between how you're setting up your environment and how the feature spec does it?

@OkayKris
Copy link

OkayKris commented Aug 31, 2023

@no-reply apologies! no test is failing, which i feel is why i'm confused. ./spec/features/collection_type_spec.rb:369 passes which means then when we recreate this on the dashboard, we should get the modal the prevents us from deleting the collection type.

this test is under the context context 'when collections exist of this type', where it 'shows unable to delete dialog and forwards to All Collections with filter applied' where we expect this to pop up:

let(:deny_delete_modal_text) do
        'You cannot delete this collection type because one or more collections of this type have already been created. ' \
        'To delete this collection type, first ensure that all collections of this type have been deleted.'

now, running docker compose -f docker-compose-koppie.yml exec -w /app/samvera/hyrax-engine app sh -c "bundle exec rspec ./spec/features/collection_type_spec.rb:355" passes which is good because that is exactly what we're trying to test for, but i can't get the same result when im physically trying to recreate this on the dashboard.

@rjkati
Copy link

rjkati commented Sep 6, 2023

This seems to be fixed on pg.nurax. As a regular user, I created a collection of the discovery only collection type. As an admin, I attempted to delete the collection type and was given a button to View collections of this type. When I clicked the View collections of this type button, I was brought to the collections screen and the filter displayed my discovery only collection.

@rjkati rjkati closed this as completed Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

9 participants