Islandora / documentation

Contains islandora's documentation and main issue queue.
MIT License
104 stars 71 forks source link

Rename `External URI` to `Preferred URI` #1286

Closed dannylamb closed 5 years ago

dannylamb commented 5 years ago

You can change it through the UI and re-export islandora_core_feature, or you can just edit these lines using a text editor:

Looking at you @rosiel :eyes:

rosiel commented 5 years ago

No, I don't think this is a good solution. Changing the label doesn't change the machine name, field_external_uri, which is present in 51 places in the islandora module. The machine name gets exposed to the user, when they're doing "add an existing field" which is a behaviour (especially with this field!) that we want to encourage. When re-using this field, it seems to default the field label to External URI anyway. The problem with this is that it's extra brain energy to wonder why we have two names for a thing, like with "Repository Item" aka islandora_object, especially if it differs between built-in vocabularies and ones they create.

If you don't want to change the machine name (and I get it, that would seriously fuck up existing installations), then may I propose that instead, we document how this is supposed to work?

And I've spent three days doing literally nothing but attempt to figure out this and other islandora workflows. When I changed my RDF mapping in order to test this, Milliner started responding with 412 and 410 errors, and nodes stopped going into Fedora periodically. I'm now on my 6th vagrant destroy in 24h, most times I vagrant up it fails halfway through (and at a different place every single time). I have Bell Fibe internet which should be blazingly fast, so IDK.

Can you tell me what I should do in order to prove that this "rewriting to the external_uri" is happening everywhere it's supposed to?

seth-shaw-unlv commented 5 years ago

Sidebar: we should probably advertise the VirtualBox snapshot capability more for dev work. Whenever I spin up a fresh box via the playbook I always login as admin and then take a snapshot. This allows me to roll back any changes should I destroy my install somehow; which means I'm up and running again in seconds rather than waiting the 30+ minutes for a fresh vagrant up.

dannylamb commented 5 years ago

@rosiel I had issues with dl'ing Solr this weekend. Killed three builds on me.

Well so we can rename it, I guess. If it's really that big of a deal. We don't really mention islandora_object by name anywhere, which is why it works if you make your own. And islandora_defaults is yours once you enable it. We don't carry it forward for you because we assume you're going to change it. So that option's still on the table, technically

And don't get bogged down by things being mentioned 51 times if they're in config. A re-export of the feature should sort all that out. You won't have to make 51 edits.

Either way, documentation is key. I imagine by the time we're done with all the issues you've raised recently we'll have built our presentation for Islandoracon and can roll it into documentation. :+1:

Let me look up all the places where that swap happens and report back.

dannylamb commented 5 years ago

EDITED BECAUSE I MISSED ONE

So here's where we use field_external_uri:

  1. The external uri is used to do lookups so that our Condition plugins don't rely on taxonomy term ID's. This happens in NodeHasTerm, MediaHasTerm, and ParentNodeHasTerm.
  2. In JsonldTypeAlterReaction, the external uri is used if present when adding rdf:types to RDF. We do this using Context.
  3. In NodeLinkHeaderSubscriber, we use check the external uri and we quite frankly shouldn't. I'm making an issue about that now.
  4. In LinkHeaderSubscriber, we generate a link header for each taxonomy term on a piece of content, and use that external uri if it's present. If not, we use the internal uri. This one is already documented here

So we only do the switcheroo when adding rdf:types based on those taxonomy terms or generating link headers.

dannylamb commented 5 years ago

And in the default config, we only utilize JsonldTypeAlterReaction in the 'All Media' and 'Content' contexts.

whikloj commented 5 years ago

So if we change the field name (which is what I think we are discussing) then it is a non-backwards compatible change, correct? We will have to cut a 2.0 release and have people (whom have a repository running) do a migration.

seth-shaw-unlv commented 5 years ago

Well, you could do an update hook to manage it and keep it a 1.1 minor release; but it does seem rather extreme at this point.

whikloj commented 5 years ago

Can you rename a field once it has data in it? Imma gonna look. 👀

seth-shaw-unlv commented 5 years ago

It is a pain to do so, but can be done.

seth-shaw-unlv commented 5 years ago

You have to create the new field, populate it, then purge the original.

rosiel commented 5 years ago

Not worth a breaking change! Let's not! (Danny wrote this ticket based on my suggestion)