Generalize DOI linker system to support other identifier types

[demiankatz]

This PR generalizes the DOI linker system so that it can carry other types of identifiers, making it more flexible. It also eliminates use of jQuery in the affected code and adds ISSN lookup support to the BrowZine identifier link handler.

The idea is that instead of passing only DOIs and using DOIs as keys for correlating links with UI elements, we now create "bundles" of identifiers keyed with an "instance index" (a simple incrementing key assigned when we generate the identifier data in the templates). This "instance index" is passed as a key to the AJAX, and these keys are maintained so that identifier-derived links can be matched back up with the UI element that triggered the lookup.

TODO

• Switch from GET to POST for data transfer
• Rename [DOI] configuration section; implement back-compatibility
• Add config setting to enable/disable lookups based on specific identifier types
• Rename everything to use more generic names
  • Javascript file
  • CSS classes
  • template file
  • AJAX handler
  • view helper
  • plugins and interface
• Update linker interface and all implementations to reflect new approach
• Fix broken tests
• Move link rendering into AJAX handler instead of Javascript for easier customization
• Discussion: do we need more backward-compatibility anywhere?
• Update changelog when merging (removal of DoiLinker plugins, replacement with IdentifierLinker plugins; replacement/renaming of associated Javascript, helpers, etc.)

[demiankatz]

This PR is now ready for review; the main question is whether we should put more work into maintaining backward compatibility with the now-obsolete DOI code, or just make a hard break and write about it in the changelog. I suspect this has not been customized much, so heavy-duty compatibility may not be worth the effort -- but if others disagree, I'd like to know! :-)

[ThoWagen]

I looked through the code changes for now but did not test it myself. But I can do so if I'm back after the holidays at 08.01.

For some first comments see below.

[demiankatz]

Thanks, @ThoWagen! I've applied most of your suggestions. See above for my comment regarding the JS loading... I couldn't find a really straightforward way to change that, but I'm open to investing more time after the break, especially if there really is a bug there.

[ThoWagen]

I looked through the code again and did some testing with the Demo resolver. Unfortunately we don't have BrowZine, so I can't test that.

There is indeed a problem when loading the results via JS (see comment) and I added another comment regarding the default configurations.

[ThoWagen]

I just finished another review for this with a couple of small suggestions.

Regarding the backward-compatibility I always have the issue with it bloating the code. However, I definitely understand why it is important in such a project. I just had the idea that we could create a separate BC module/theme and move all the BC logic there so that the base modules stay as clean as possible. Do you think that might be possible and sensible to do?

[ThoWagen]

A closing bracket is missing here

Suggested change

	<img<?=$this->htmlAttributes(['src' => $link['icon'], 'class' => 'identifier-link-icon icon-link__icon'])?>
	<img<?=$this->htmlAttributes(['src' => $link['icon'], 'class' => 'identifier-link-icon icon-link__icon'])?>>

And should we add an "alt" attribute here?

[demiankatz]

Good catch! I'm surprised that browsers are so tolerant of that missing character; it worked fine for me even with the error. In any case, it's fixed now, and I added a blank alt attribute (since these icons don't add meaningful information beyond the link label that follows them).

[ThoWagen]

Makes sense. But should we add aria-hidden="true" for accessibility since alt is blank?

[demiankatz]

Thanks for the latest review, @ThoWagen -- good catches and ideas, and I'll try to work through it all later this week!

Regarding the idea of a "legacy" or "backward-compatibility" module, this has definitely crossed my mind before. I think the challenge is deciding the scope of what would go into the module. Some back-compatible behaviors are easily separated and moved to a module (e.g. legacy route definitions, legacy wrapper classes, legacy aliases). Others are not (e.g. fallbacks to previous configuration names during lookup). So there are some cases where separating things would improve readability and reduce complexity in the core code... and other cases where trying to separate things would actually add complexity and make things less straightforward. This split has prevented me from moving forward with the idea in the past, which is not to say I'm necessarily opposed to it in the future. Maybe a place to start would be a community conversation about how long we maintain different types of legacy behaviors, which could lead to a strategy about when we can safely deprecate/remove things, when we might want to keep legacy items around forever (e.g. routes, for ongoing availability of historical links), etc. If you like this idea, I'm happy to add it to a future Community Call agenda.

[ThoWagen]

I believe discussing this topic in a community call makes sense! I also wonder if anyone has different ideas how to mark logic as BC so that it can safely be removed after some time.

And in my opinion it would still be beneficial to have an BC module only for those cases where it does not increase complexity and redundancy.

[demiankatz]

I believe discussing this topic in a community call makes sense! I also wonder if anyone has different ideas how to mark logic as BC so that it can safely be removed after some time.

And in my opinion it would still be beneficial to have an BC module only for those cases where it does not increase complexity and redundancy.

Sounds good -- I have put this on the agenda for the February 4, 2025 call.