views: add signposting

[fenekku]

See inveniosoftware/rfcs#73

[fenekku]

failing tests have to do with seemingly unrelated code:

FAILED tests/requests/test_user_moderation_actions.py::test_user_moderation_approve
FAILED tests/services/test_rdm_service.py::test_search_sort_verified_enabled

Will check later if any actual connection.

[lnielsen]

Thanks a lot for implementing all this. It's very relevant and timely for InvenioRDM so fully support this gets.

[lnielsen]

I think this is going to hit the database, so it should be using some sort of cache to not get a performance hit. @ntarocco was looking into similar issues with the search results.

[fenekku]

Looks like get_vocabulary_props should do the job.

[lnielsen]

Comment: I understand that the current link generation doesn't work for this use case, but I'm concerned that introducing these two methods will muddy the picture of where link generation should happen.

Would a possible solution be to return a URITemplate in the links or linktpls which could be used to generate the links you need. I.e. these two methods would take as input a URItemplate which was generated by the service layer and simply render the string.

[fenekku]

Hmm this is a longer discussion. The practical piece I got from the comment is to rely on the already generated links that are part of the record projection that is being serialized into json+linkset in the schema.py. I've adapted the code to rely on that e.g., https://github.com/inveniosoftware/invenio-rdm-records/pull/1404/files#diff-32235a765fef3b5ef2104531e468805d799d3d8b9db5b501d09295b4a365b4eaR33 👍 .
The download link is not part of the provided serialized links, so I kept the use of download_url_for there. I also kept record_url_for in the header link generation, since we don't necessarily have access to those links there either.

We can have a longer discussion about link generation in a dedicated ticket. Main desires for me are (to repeat some from the docs from urls.py):

• avoid already potential conflicts between routes defined in APP_RDM_ROUTES and routes defined in RDMRecordServiceConfig.links_item (link generation is already muddied)
• be able to generate links anywhere with the same call pattern and different arguments i.e. like url_for but also from either UI or API application (in or out of request context). Maybe name it inveniordm_url_for
• have well defined endpoint names to support that and the configuration of URLs

[fenekku]

Working on it

[fenekku]

Changes completed. Re-check :)

[fenekku]

Looks like get_vocabulary_props should do the job.

[fenekku]

Hmm this is a longer discussion. The practical piece I got from the comment is to rely on the already generated links that are part of the record projection that is being serialized into json+linkset in the schema.py. I've adapted the code to rely on that e.g., https://github.com/inveniosoftware/invenio-rdm-records/pull/1404/files#diff-32235a765fef3b5ef2104531e468805d799d3d8b9db5b501d09295b4a365b4eaR33 👍 .
The download link is not part of the provided serialized links, so I kept the use of download_url_for there. I also kept record_url_for in the header link generation, since we don't necessarily have access to those links there either.

We can have a longer discussion about link generation in a dedicated ticket. Main desires for me are (to repeat some from the docs from urls.py):

• avoid already potential conflicts between routes defined in APP_RDM_ROUTES and routes defined in RDMRecordServiceConfig.links_item (link generation is already muddied)
• be able to generate links anywhere with the same call pattern and different arguments i.e. like url_for but also from either UI or API application (in or out of request context). Maybe name it inveniordm_url_for
• have well defined endpoint names to support that and the configuration of URLs

[fenekku]

Rebased + tests updated for new application/ld+json mimetype + take into account new files structure (fixes

"href": download_url_for(pid_value=obj["id"], filename=entry["key"]),
TypeError: string indices must be integers

and other related errors.

[fenekku]

Just need to check the to_dict() representation for search before I put this back out of draft. (Although, the "application/linkset+json" serialization is not really meant to be search-serialized anyway). By end of tomorrow my time, it should be resolved.

I currently suspect that most serialization tests are not using a proper to_dict() representation as input and therefore are potentially doing unnecessary work (but I may be off). That's why I included a static full_record_to_dict fixture and will check the search use case. I'd be happy to refactor the other serializations if that is the case. Thanks to @chokribr for bringing this to my attention when checking the functionality.

[fenekku]

The full_record / minimal_record in root conftest.py are (were) meant as record creation input from the REST API / service side of things, but they have been altered over time. Relying on them is more confusing/clunky than not, ergo the creation of clear to_dict() fixtures here.

In particular, the usage of the old fixture may have given the wrong impression of what data is available from the serialization input and what data must indeed be fetched from DB/OS. If I am correct, then the datacite serializer for example (and most likely others) could be made much more performant by skipping subjects_service.read_many() and vocabulary_service.read_many(system_identity, "licenses", ids) calls for instance since the data is already available in the obj to serialize at this point. Hopefully those are not there for a weird edge case I am not aware-of 😄 .

[github-actions]

This PR was automatically marked as stale.