Add a generic ItemPageLicenseFieldComponent#3338
Add a generic ItemPageLicenseFieldComponent#3338abelgomez wants to merge 24 commits intoDSpace:mainfrom
Conversation
|
Hi @abelgomez, |
|
This PR should be ready for review. Sorry for the long list of commits: when changing from the Since this PR delegates the rendering of CC licenses to the ItemPageCcLicenseFieldComponent, this PR is affected by DSpace/DSpace#9882 and #3165. I opted to keep ItemPageCcLicenseFieldComponent almost as is to avoid breaking any existing installation that relies on it. |
|
@abelgomez : Could you rebase your changes directly on |
|
As far as I remember, that was exactly the problem that caused the long list of commits: I rebased main onto my branch but GitHub does not detect that those commits were already on "main". I'll try to fix it, and if I fail, I'll close the PR and open a clean one. |
…er than ItemPageCcLicenseFieldComponent (fixes DSpace#3170)
…nComponent) (fixes DSpace#3167)
* Trailing spaces * Missing trailing commas * Imports sorting * Equalities and typing
…er than ItemPageCcLicenseFieldComponent (fixes DSpace#3170)
…nComponent) (fixes DSpace#3167)
* Trailing spaces * Missing trailing commas * Imports sorting * Equalities and typing
This PR relates to DSpace#3165 and DSpace/DSpace#9882.
|
I started from scratch, and cherry-picked the interesting commits. Let me know if you want me to also combine several commits into one for a cleaner history. |
|
Hi @abelgomez, |
|
@abelgomez : If you wish this to be considered for 9.0, we'll need you to resolve the merge conflicts in this PR. Our merger deadline for new feature PRs is March 28. So, any new feature PR not review, approved and merged by then will not be in 9.0. |
|
Hi @tdonohue, sure, I can resolve the conflicts. I was waiting to see what happens with other closely related open PRs (DSpace/DSpace#9882 and #3165). I think that DSpace/DSpace#9882 (which is kind of a pre-requisite) can be easily merged as a starting point. Regarding this PR I can handle it today, since it's a trivial fix that only affects the English language file. Once I check why #3165 is failing for you, would it be possible to include both PRs in the same release? |
|
Hi @alexandrevryghem , I have incorporated the changes you proposed. This PR should be ready for review and merge. Keep in mind that, since I refactored the code to parse/validate CC URIs to a separate utility file as you suggested, this PR has conflicts with #3165. Thus, if the plan is to include both, I'll need a small margin to reconcile both PRs when the first one is merged (whichever it will be). |
|
@abelgomez: I think it will be easier to merge #3165 first since it's smaller. Now that I have properly compared both PRs it does look like the |
|
Ho @alexandrevryghem , as you wish. The point of this PR is to directly show the rights information in the simple page without requiring to modify the templates. For me, seems weird that, by default, DSpaces show a badge and the license information if Of course, institutions can customize what is shown or hidden by customizing templates, but this PR aims at showing relevant information for those who don't want to customize the templates. Based on my experience, making big customizations to the Angular code makes upgrades difficult and time-consuming, since every major version typically includes several breaking changes, and institutions need to reconcile their templates with the "base" code. Thus, I think that some institutions would like to avoid as many customizations as possible. Cheers! |
|
Hi @abelgomez, |
|
Hi @abelgomez, |
|
Hi @abelgomez, |
|
Hi @abelgomez, the new feature you developped will be of interest for us as it also corrrects a problems when using entities in DSpace (the CC icon and text not displayed on Publication type Item). |
|
Hi @pilasou , Thanks for letting me know. This PR is not "abandoned", I plan to resolve the current conflicts and keep this PR in sync with the master branch and let the DSpace team decide whether it should be merged or not. I simply haven't had the time to do it yet. Cheers, Abel |
|
Thanks @abelgomez, no worries, i am not a developper so I cannot make code review, but once you resolve conflit, I will certainly test it! |
|
@abelgomez : We are just beginning a more detailed review process for 10.0 for all new feature PRs. If you can rebase this on latest |
|
@tdonohue , done! |
4 participants
References
dc.rightsis a CC license #3170Description
This PR adds a new ItemPageLicenseFieldComponent, which allows showing generic rights information. If the component detects that the rights information points to a Creative Commons license, it delegates the rendering to the existing ItemPageCcLicenseFieldComponent.
If the item contains the same numbers of URIs than license names/descriptions, the links will be associated to the names/descriptions in the order they appear. If not, all fields will be explicitly listed (see example below).
Instructions for Reviewers
Create an item with the following properties:
dc.rights.uri: https://www.example.com/licensedc.rights: All rights reservedLicense should be rendered as a license text with a link:
Create an item with the following properties:
dc.rights.uri: https://www.example.com/license1dc.rights.uri: https://www.example.com/license2dc.rights: License 1dc.rights: License 2Licenses should be rendered as list of licenses with links:
Create an item with the following properties:
dc.rights.uri: https://www.example.com/license1dc.rights.uri: https://www.example.com/license2dc.rights: License 1dc.rights: License 2dc.rights: License 3Licenses should render all fields explicitly (and URIs will be rendered as links), since no matching can be established between the license names and the links:
Create an item with the following properties:
dc.rights.uri: https://creativecommons.org/licenses/by-nc-sa/4.0/dc.rights: CC BY-NC-SA 4.0License will be rendered as in DSpace 9 using ItemPageCcLicenseFieldComponent:
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lintnpm run check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.