Skip to content

clc: add clc sync message #483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jrcastro2
Copy link
Contributor

@jrcastro2 jrcastro2 commented May 16, 2025

image

invenio.cfg Outdated
Comment on lines 520 to 521
"cds_rdm/records/manage_menu.html" if template == "invenio_app_rdm/records/details/side_bar/manage_menu.html" else template
for template in DEFAULT_SIDE_BAR_TEMPLATES
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the if condition neede?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because the idea is to replace the existing manage_menu.html template only. The goal is to avoid overriding the entire template which we could do, but I simply want to add new data attributes.

@jrcastro2 jrcastro2 force-pushed the add-clc-message branch 2 times, most recently from 0d508fb to 179eed5 Compare May 16, 2025 11:38
@jrcastro2 jrcastro2 moved this to In review 🔍 in Sprint Q2/2025 🌻 May 19, 2025
@@ -38,7 +37,7 @@ class CLCSyncPermissionPolicy(BasePermissionPolicy):
"""Permission policy for CLCSync."""

can_create = [CLCExporter(), Administration(), SystemProcess()]
can_read = [CLCExporter(), Administration(), SystemProcess()]
can_read = [CLCExporter(), Administration(), SystemProcess(), AnyUser()]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does Any User need to read?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to retrieve if the record is synced or not on each landing page, for any record, for any user, to be able to conditionally display the new banner. @jrcastro2 you can probably remove all the other permissions given the AnyUser.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants