Fix/broken term

[adam-vessey]

What does this Pull Request do?

Avoids creating a broken taxonomy term on installation during site installation proper.

• Related GitHub Issue: Fixes #13

• Other Relevant Links: (Google Groups discussion, related pull requests, Release pull requests, etc.)

What's new?

A in-depth description of the changes made by this PR. Technical details and possible side effects.

• Changes x feature to such that y
• Added x
• Removed y
• Does this change add any new dependencies?
• Does this change require any other modifications to be made to the repository (i.e. Regeneration activity, etc.)?
• Could this change impact execution of existing code?

How should this be tested?

A description of what steps someone could take to:

• Reproduce the problem you are fixing (if applicable)
• Test that the Pull Request does what is intended.
• Please be as detailed as possible.
• Good testing instructions help get your PR completed faster.

Documentation Status

• Does this change existing behaviour that's currently documented?
• Does this change require new pages or sections of documentation?
• Who does this need to be documented for?
• Associated documentation pull request(s): or documentation issue

Additional Notes:

Any additional information that you think would be helpful when reviewing this PR.

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

[rosiel]

Maybe I jumped the gun on this... but I confirmed the problem that a term was being created without a URI. However with this branch in my starter-site, using commit 3aca206, no Mirador term was created at all.

[adam-vessey]

@rosiel : Would be up to adding/ensuring the new islandora_mirador_tags migration is included in the set of whatever migrations run during site installation... "traditionally", this was done with the --group=islandora bit, but it seems like things might've moved to target specific migrations?:

• isle-dc:
  • https://github.com/Islandora-Devops/isle-dc/blob/e305e39308f391259c0d0e57252e7557c20cba16/Makefile#L581 and
  • https://github.com/Islandora-Devops/isle-dc/blob/e305e39308f391259c0d0e57252e7557c20cba16/Makefile#L615
• playbook:
  • https://github.com/Islandora-Devops/islandora-playbook/blob/1c34012a9bd4634448e04181a9497ccd42eecbe7/post-install.yml#L58-L63

[rosiel]

Dang, of course.

I don't think there was a desire to go for specific migrations rather than calling as a group, but the migrations currently aren't all tagged 'islandora'. Secondly, there's a when clause in the Playbook that insinuates that sometimes the FITS migration shouldn't run. I'm hesitant to take that out because I don't want to get burned.

So in short I think we were lazy and listed migrations, rather than it being a deliberate choice. But now that we've brought it up, there's a lot of work we should do to harmonize the READMEs and the code.

[adam-vessey]

@rosiel : For FITS in the playbook, it was more that the starter site was the first thing using it in the play, so other environments had to default to not doing it (as they would've not had the migration present), and it was following the pattern of explicitly referencing the migrations... thinking, could similarly do something like https://github.com/Islandora-Devops/islandora-playbook/pull/264 to allow the starter site to start using --tag=islandora to run the migrations, while leaving any other environments alone, by default.

Is somewhat more difficult in isle-dc... https://github.com/Islandora-Devops/isle-dc/pull/342 should do the trick for starter site stuff; however technically, it would try to run the islandora_defaults_tags and islandora_tags twice. Not the end of the world, as it shouldn't do anything the second time it tries to run them, but it's one of those things that just seems kind of... less-than ideal? That said, unsure how to do things nice and generally there, as it's somewhat dependent whatever site's configuration that is installed, and with a single project trying to support multiple configurations, is somewhat rather unwieldly.

[jordandukart]

This looks fine to me, tested in the following scenarios:

On a box that had islandora_mirador installed I removed the URI of the term to facilitate a "broken" scenario.

Navigating to admin/reports/status#warning yields: .

Similarly, deleting the term and re-enabling the module shows the following: .

After running the migration the term exists as we expect:

 [notice] Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - done with 'islandora_mirador_tags'```