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

Remove requirement for resource type. #11

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

rosiel
Copy link
Contributor

@rosiel rosiel commented Feb 24, 2022

Pursuant to Islandora/documentation#2052, this PR changes system actions to not require the Resource Type field. It also makes the field multivalued and not required.

@noahwsmith
Copy link
Contributor

I think this will break a number of assumptions of the https://github.com/Islandora-Devops/islandora_starter_theme but I'd have to really test it to find out. I will try to get feedback for this soon...

@rosiel
Copy link
Contributor Author

rosiel commented Mar 8, 2022

There is no reference to the regex resource.type in the starter or base theme.

@noahwsmith
Copy link
Contributor

@dannylamb could you please take a look at this?

@wgilling wgilling self-requested a review April 13, 2022 18:00
@seth-shaw-unlv seth-shaw-unlv self-requested a review May 4, 2022 17:56
@rosiel
Copy link
Contributor Author

rosiel commented May 4, 2022

Testing Instructions:

  1. Get a copy of isle-dc.
  2. Edit this line to git clone -b 2052-use-pr-branch https://github.com/islandora-devops/islandora-sandbox (the -b part is new)
  3. Since 2052-use-pr-branch points to my PR branch, doing a launch of isle dc should work to pull in this PR.

@seth-shaw-unlv
Copy link
Contributor

It looks like something else may be missing in this setup. I've run make clean; make local-install-profile with the Makefile update to pull the branch, but it keeps failing on SOLR:

In SolrCommandHelper.php line 119:

  Unknown server default_solr_server




ERROR: Could not generate SOLR config.zip!
In Drupal, check Configuration -> Search API -> SOLR Server, and use the
+ Get config.zip option which should give you information into the actual error.


make[1]: *** [Makefile:167: solr-cores] Error 1
make[1]: Leaving directory '/home/sshaw/isle-dc'
make: *** [Makefile:398: local-install-profile] Error 2

Which version of isle-dc or the image tags are you using, @rosiel?

@noahwsmith
Copy link
Contributor

This may be hard to test before this PR is accepted, which addresses some order of operation issues
Islandora-Devops/isle-dc#248

Removing resource type from implicit parent taxonomy uri lookup
@noahwsmith
Copy link
Contributor

Previous change to address context change to remove hardcode requirement for resource type

@noahwsmith
Copy link
Contributor

This Issue was picked as part of an IslandoraCon 2022 Use-A-Thon project - more info here https://docs.google.com/document/d/17rkEzgFjxv5WCVmt-MqZLXYcvO4z658zs5hG9SP2FmQ/edit#

Current status is that I have worked through all of these settings in my local, and achieved a full working state that does not require Resource Type (or use it for any context). I'd like to test this on a rebuilt local before I turn this over to others for testing...

@noahwsmith
Copy link
Contributor

noahwsmith commented Aug 8, 2022

Updating Rosie's previous review instructions:

  1. Get a copy of isle-dc.
  2. Edit sample.env to read TAG=1.0.7 instead of TAG=1.0.0-alpha-16 if it does not already.
  3. Edit this line to git clone https://github.com/islandora-devops/islandora-sandbox -b 2052-use-pr-branch /tmp/codebase (the -b is changed from main).
  4. Since 2052-use-pr-branch points to my PR branch, doing a launch of isle dc should work to pull in this PR. Run make local
  5. To get some sample data, run a subsequent make demo_content

Test criteria: ensure that all types of content display as expected, that search is not broken, and that all ingests from the demo_content import succeed.

My test results: everything works great except for Newspapers. I will go another round on those, especially since I see the same issues in @ysuarez 's new install and I can work on both things at once.

@@ -17,7 +17,6 @@ conditions:
parent_reference_field: field_member_of
model_uri: 'http://vocab.getty.edu/aat/300242735'
logic: and
uri: 'http://purl.org/dc/dcmitype/Collection'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR is currently failing to do a drush site-install, i'm testing whether this is the line that's causing (sorry for misremembered wording) "Config could not be enabled."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Rosie - I'm not able to replicate this. Did you run make local which contains docker-compose exec -T drupal with-contenv bash -lc "drush si -y islandora_install_profile_demo --account-pass $(shell cat secrets/live/DRUPAL_DEFAULT_ACCOUNT_PASSWORD)" or a different function?

@noahwsmith
Copy link
Contributor

Ok, part of the Newspaper problem was related to Ghostscript not being present on the 1.0.0-alpha-16 containers. This is fixed in latest containers, so I pushed this change and will re-test with the latest containers.

Islandora-Devops/isle-dc#285

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants