Configure share link expiration date required (frontend part) #3187

mkloeppe · 2025-09-22T08:30:05Z

❤️ Thank you for your contribution!

Description

This is the frontend part of this PR.
It changes the help text depending on the configuration and adds error styles if the expiration date is required.
This PR has to be merged AFTER inveniosoftware/invenio-rdm-records#2169

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

I'm aware of the code of conduct.
I've created logical separate commits and followed the commit message format.
I've added relevant test cases.
I've added relevant documentation.
I've marked translation strings.
I've identified the copyright holder(s) and updated copyright headers for touched files (>15 lines contributions).
I've NOT included third-party code (copy/pasted source code or new dependencies).
- If you have added third-party code (copy/pasted or new dependencies), please reach out to an architect on Discord.

Frontend

I've followed the CSS/JS and React guidelines.
I've followed the web accessibility guidelines.
I've followed the user interface guidelines.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

You license your contribution under the same terms as the current repository’s license.
You agree that you have the right to license your contribution under the current repository’s license.

karkraeg · 2025-09-23T07:04:21Z

sorry for my non-conventional commit message. I will think about it next time!

max-moser

the logic looks good to me, i just have a few minor suggestions about consistency and intentionality 🙂

max-moser · 2025-10-09T14:07:40Z

invenio_app_rdm/records_ui/templates/semantic-ui/invenio_app_rdm/records/deposit.html

           value='{{ permissions | tojson }}'>
  {%- endif %}

+<span id="deposit-share-btn-require-link-expiration"


Suggested change

<span id="deposit-share-btn-require-link-expiration"

<input type="hidden" id="deposits-share-btn-require-link-expiration"

i'd keep the indentation consistent with other elements – in my experience, inconsistent indenting makes HTML very hard to read very quickly.

the change from <span> to <input> + ID change (deposit- -> deposits-) is also mostly for consistency with the other config data elements on the deposit form.

max-moser · 2025-10-09T14:08:09Z

invenio_app_rdm/records_ui/templates/semantic-ui/invenio_app_rdm/records/detail.html



 {%- block page_body %}
+<span id="detail-share-btn-require-link-expiration"


Suggested change

<span id="detail-share-btn-require-link-expiration"

<span id="detail-share-btn-require-link-expiration"

note: in contrast to the deposit form, i couldn't quickly identify a trend on the landing page regarding type and ID of config elements

max-moser · 2025-10-09T14:20:03Z

...ets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/CreateAccessLink.js

  }
+  checkIfIsAccessLinksExpirationRequired = () => {
+    return (
+      document.getElementById("deposit-share-btn-require-link-expiration")?.dataset


Suggested change

document.getElementById("deposit-share-btn-require-link-expiration")?.dataset

document.getElementById("deposits-share-btn-require-link-expiration")?.dataset

max-moser · 2025-10-09T14:27:49Z

...ic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksSearchResultContainer.js

  render() {
    const { results, record, onItemAddedOrDeleted, onPermissionChanged } = this.props;
    const { loading, error } = this.state;
+    const isLinkExpirationDateError = this.checkIfLinkExpirationError(error);


Suggested change

const isLinkExpirationDateError = this.checkIfLinkExpirationError(error);

const hasLinkExpirationDateError = this.checkIfLinkExpirationError(error);

max-moser · 2025-10-09T14:27:59Z

...ic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/LinksSearchResultContainer.js


        <Table color="green">
          <CreateAccessLink
+            hasError={isLinkExpirationDateError}


Suggested change

hasError={isLinkExpirationDateError}

hasError={hasLinkExpirationDateError}

max-moser · 2025-10-09T14:39:39Z

...ets/semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessLinks/CreateAccessLink.js

+};
+
+CreateAccessLink.defaultProps = {
+  hasError: false,


i feel there's a slight discrepancy between the naming and actual use of the property here:
the naming suggests the presence/absence of any generic error, but the use is limited only to the expiration date

i'd prefer an alternative, like the following suggestions:

rename the variable to something more narrow like hasExpirationDateError, or

change the nature of the variable, e.g. into a list named errors that contains the names of all fields with validation errors... or a dictionary of the shape {fieldName: errorMessage, ...}

the second option is a bit more flexible but probably overkill, the first one is simpler but a bit more limited.
note that these two are just the first ideas that came to my mind, i'm fine with other alternatives as long as they make the intentionality a bit clearer!

max-moser

the logic looks good to me, i just have a few minor suggestions about consistency and intentionality 🙂

max-moser · 2025-10-13T13:23:53Z

I just remembered that there is another spot that needs to know about required expiration dates for secret links:
The "guest access request" (which generates secret links & sends them to the guest's email address), i.e. AccessRequestTimelineEdit.js & co

mkloeppe · 2025-10-14T11:09:16Z

So, to get the config variable to the <AccessRequestTab> I lifted it to the parent component (<ShareModal>). When the config variable is true in <AccessRequestExpirationSelect> the "never expires" option is being removed. I additionally adjusted all the feedback from before.

max-moser

thanks a lot for the update, and sorry for the long wait!
from my perspective, just one more comment about an else if – other than that, I'm setting up a new my-site to quickly sanity check, and will get back after

...semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessRequests/AccessRequestsTab.js

max-moser

LGTM, thanks for all the work!

max-moser · 2025-10-23T13:10:53Z

...semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessRequests/AccessRequestsTab.js

    }
+    else if (settings["secret_link_expiration"] === 0 && isAccessLinksExpirationRequired) {


Suggested change

}

else if (settings["secret_link_expiration"] === 0 && isAccessLinksExpirationRequired) {

} else if (settings["secret_link_expiration"] === 0 && isAccessLinksExpirationRequired) {

looks like the linter wants it to be on the same line

* the new configuration variable RDM_RECORDS_REQUIRE_SECRET_LINKS_EXPIRATION adds the option to make the expiration date required * added error messages improve the user experience.

karkraeg added this to vNext Sep 23, 2025

karkraeg moved this to To test in vNext Sep 23, 2025

karkraeg mentioned this pull request Sep 29, 2025

feat: Add configuration to make expiration date of share links required inveniosoftware/invenio-rdm-records#2169

Open

5 tasks

karkraeg force-pushed the configure-share-link-expiration-date-required-frontend branch 4 times, most recently from 4621e0e to 096adaa Compare September 30, 2025 06:29

karkraeg requested a review from max-moser October 2, 2025 05:19

utnapischtim approved these changes Oct 3, 2025

View reviewed changes

max-moser reviewed Oct 9, 2025

View reviewed changes

mkloeppe force-pushed the configure-share-link-expiration-date-required-frontend branch 2 times, most recently from 096adaa to ab0e4dc Compare October 13, 2025 12:08

mkloeppe force-pushed the configure-share-link-expiration-date-required-frontend branch from ab0e4dc to 4ab69ec Compare October 14, 2025 09:49

mkloeppe closed this Oct 14, 2025

mkloeppe force-pushed the configure-share-link-expiration-date-required-frontend branch from 4ab69ec to 2186821 Compare October 14, 2025 10:37

github-project-automation bot moved this from To test to To release 🤖 in vNext Oct 14, 2025

mkloeppe reopened this Oct 14, 2025

mkloeppe force-pushed the configure-share-link-expiration-date-required-frontend branch 2 times, most recently from 264c363 to bb007ea Compare October 22, 2025 14:59

max-moser reviewed Oct 22, 2025

View reviewed changes

...semantic-ui/js/invenio_app_rdm/landing_page/ShareOptions/AccessRequests/AccessRequestsTab.js Outdated Show resolved Hide resolved

max-moser approved these changes Oct 23, 2025

View reviewed changes

mkloeppe force-pushed the configure-share-link-expiration-date-required-frontend branch from 2baad3e to cabae40 Compare October 30, 2025 09:03

feat(shareBtn): handle optional required expiration

df90a74

* the new configuration variable RDM_RECORDS_REQUIRE_SECRET_LINKS_EXPIRATION adds the option to make the expiration date required * added error messages improve the user experience.

mkloeppe force-pushed the configure-share-link-expiration-date-required-frontend branch from cabae40 to df90a74 Compare October 30, 2025 09:53

	<span id="deposit-share-btn-require-link-expiration"
	<input type="hidden" id="deposits-share-btn-require-link-expiration"



		{%- block page_body %}
		<span id="detail-share-btn-require-link-expiration"

	document.getElementById("deposit-share-btn-require-link-expiration")?.dataset
	document.getElementById("deposits-share-btn-require-link-expiration")?.dataset

	const isLinkExpirationDateError = this.checkIfLinkExpirationError(error);
	const hasLinkExpirationDateError = this.checkIfLinkExpirationError(error);

	hasError={isLinkExpirationDateError}
	hasError={hasLinkExpirationDateError}

		}
		else if (settings["secret_link_expiration"] === 0 && isAccessLinksExpirationRequired) {

	}
	else if (settings["secret_link_expiration"] === 0 && isAccessLinksExpirationRequired) {
	} else if (settings["secret_link_expiration"] === 0 && isAccessLinksExpirationRequired) {

Configure share link expiration date required (frontend part) #3187

Are you sure you want to change the base?

Configure share link expiration date required (frontend part) #3187

Uh oh!

Conversation

mkloeppe commented Sep 22, 2025 • edited by karkraeg Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

karkraeg commented Sep 23, 2025

Uh oh!

max-moser left a comment

Choose a reason for hiding this comment

Uh oh!

max-moser Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

max-moser Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

max-moser Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

max-moser Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

max-moser Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

max-moser Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

max-moser left a comment

Choose a reason for hiding this comment

Uh oh!

max-moser commented Oct 13, 2025

Uh oh!

mkloeppe commented Oct 14, 2025

Uh oh!

max-moser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

max-moser left a comment

Choose a reason for hiding this comment

Uh oh!

max-moser Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mkloeppe commented Sep 22, 2025 •

edited by karkraeg

Loading