Skip to content
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.

Sign up for GitHub

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

Jump to bottom

records/macros/detail.html: Allow funding entry with award number only #2912

Merged
merged 6 commits into from
Jan 11, 2025
Merged

records/macros/detail.html: Allow funding entry with award number only #2912

merged 6 commits into from
Jan 11, 2025

Conversation

karkraeg
Copy link
Member

@karkraeg karkraeg commented Nov 15, 2024 •
edited
Loading

❤️ Thank you for your contribution!

Description

In order to display funding entries with only an award number after inveniosoftware/invenio-vocabularies#429 has been merged this PR changes the display logic.

Checklist

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

Reminder

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

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

@karkraeg karkraeg mentioned this pull request Nov 15, 2024
Merged
4 tasks
tmorrell
tmorrell reviewed Nov 15, 2024
View reviewed changes
Copy link
Contributor

@tmorrell tmorrell left a comment

Choose a reason for hiding this comment

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

I'd lean toward putting the title first and simplifying the template.

invenio_app_rdm/records_ui/templates/semantic-ui/invenio_app_rdm/records/macros/detail.html Show resolved Hide resolved
invenio_app_rdm/records_ui/templates/semantic-ui/invenio_app_rdm/records/macros/detail.html Outdated Show resolved Hide resolved
@karkraeg
Copy link
Member Author

Funder + Number

image

Two funding entries, second one with URL

image

No award number

image

Award number + title

image

Only award number

image

Just a funder name

image

@karkraeg
Copy link
Member Author

Hi @tmorrell , is this the way you were thinking?

@tmorrell
Copy link
Contributor

Could you add one or two screenshots?

@wgresshoff
Copy link

wgresshoff commented Nov 22, 2024 •
edited
Loading

Perhaps the wrong place here (surely the wrong place here!) but it's just a thought: did someone bother to look if the change has implications on OAI-PMH output? If the transformers rely on number AND title instead of number OR title?!

@tmorrell
Copy link
Contributor

@wgresshoff Funding isn't serialized in the default dc oai output. The DataCite output should be alright assuming I understand how missing is working. @karkraeg if you've got a test instance up it would be worth running through the export formats on an example record and making sure they don't break.

@tmorrell
Copy link
Contributor

The serialized formats are fine. Since inveniosoftware/invenio-vocabularies#429 has been released it would be great to get this change merged so the award number shows up consistently.

There seems to be a bit of a spacing issue around the award number:
Screenshot 2024-12-16 at 11 42 43 AM

I've just made a suggestion that re-works the spacing. I think we should merge after that change is added, unless folks want to do more re-designing.

Here is the UI after the change:
Screenshot 2024-12-16 at 12 08 55 PM
Screenshot 2024-12-16 at 12 09 28 PM
Screenshot 2024-12-16 at 12 10 02 PM
Screenshot 2024-12-16 at 12 10 28 PM
Screenshot 2024-12-16 at 12 11 02 PM

@karkraeg
Copy link
Member Author

karkraeg commented Dec 20, 2024 •
edited
Loading

The serialized formats are fine. Since inveniosoftware/invenio-vocabularies#429 has been released it would be great to get this change merged so the award number shows up consistently.

Sorry Tom that I missed that... Looks nice this way, I'd opt for merging this.

@tmorrell
Copy link
Contributor

Could you make the change #2912 (comment) in your fork? Then I should be able to merge it.

@karkraeg
Copy link
Member Author

karkraeg commented Jan 6, 2025

Tested it, looks nice this way!

@tmorrell tmorrell merged commit 50ec8eb into inveniosoftware:master Jan 11, 2025
4 checks passed
slint added a commit to slint/zenodo-rdm that referenced this pull request Jan 16, 2025
@slint
installation: bump dependencies
9f3db82 
📁 invenio-app-rdm (13.0.0b1.dev24 -> 13.0.0b1.dev27 🌈)

    📦 release: v13.0.0b1.dev27
    Revert "config: add record and membership comment notifications"
    📦 release: v13.0.0b1.dev26
    config: add record and membership comment notifications
    records/macros/detail.html: Allow funding entry with award number only (inveniosoftware/invenio-app-rdm#2912)
    bug: remove unnecessary p tag
    bug: close unclosed div
    release: v13.0.0b1.dev25
    doi: handle UI for optional DOI feature
    theme: put banner message below other objects
    pages: do not create a static page if it already exists

📁 invenio-communities (17.7.1 -> 17.8.1 🌈)

    📦 release: v17.8.1
    Revert "notifications: add comment notification for record and membership"
    📦 release: v17.8.0
    organizations: updated knownOrganizations to use id
    notifications: add comment notification for record and membership

📁 invenio-mail (2.1.1 -> 2.2.0 🌈)

    release: v2.2.0
    Add exception handling and retry mechanism (inveniosoftware/invenio-mail#67)
    * Add exception handling and retry mechanism.
    * Retry with exponential backoff with random jitter to prevent "Thundering herd problem".

📁 invenio-rdm-records (16.4.2 -> 16.6.1 🌈)

    📦 release: v16.6.1
    Revert "bug: add custom comment notification for record/draft requests"
    📦 release: v16.6.0
    bug: add custom comment notification for record/draft requests
    affiliations: update AffiliationsField value
    fix: use uow not self.uow
    serializers: bibtex: Conference paper not falling back to proceedings
    serializers: bibtex: Conference proceeding to proceedings
    serializers: bibtex: year and month using publication date
    📦 release: v16.4.2
    rights: fix serialize condition for controlled license
    rights: fix serialize condition for controlled license
    release: v16.5.1
    optional-doi: fix validation check when user needs a DOI
    pids: add manage permission to be able to skip doi transitions checks
    release: v16.5.0
    doi: handle UI for optional DOI feature

📁 invenio-theme (3.5.1 -> 3.5.2 🐛)

    release: v3.5.2
    form: some UI fixed for the deposit form

📁 invenio-userprofiles (3.0.1 -> 3.0.2 🐛)

    release: v3.0.2
    fixed 404 issue when pressing cancel

📁 invenio-vocabularies (6.9.0 -> 6.11.0 🌈)

    release: v6.11.0
    names: allow to add the acronym
    release: v6.10.1
    names: drop unique constraint on internal id
    release: v6.10.0
    names: add internal id to identify the names
slint added a commit to zenodo/zenodo-rdm that referenced this pull request Jan 16, 2025
@slint
installation: bump dependencies
c05855c 
📁 invenio-app-rdm (13.0.0b1.dev24 -> 13.0.0b1.dev27 🌈)

    📦 release: v13.0.0b1.dev27
    Revert "config: add record and membership comment notifications"
    📦 release: v13.0.0b1.dev26
    config: add record and membership comment notifications
    records/macros/detail.html: Allow funding entry with award number only (inveniosoftware/invenio-app-rdm#2912)
    bug: remove unnecessary p tag
    bug: close unclosed div
    release: v13.0.0b1.dev25
    doi: handle UI for optional DOI feature
    theme: put banner message below other objects
    pages: do not create a static page if it already exists

📁 invenio-communities (17.7.1 -> 17.8.1 🌈)

    📦 release: v17.8.1
    Revert "notifications: add comment notification for record and membership"
    📦 release: v17.8.0
    organizations: updated knownOrganizations to use id
    notifications: add comment notification for record and membership

📁 invenio-mail (2.1.1 -> 2.2.0 🌈)

    release: v2.2.0
    Add exception handling and retry mechanism (inveniosoftware/invenio-mail#67)
    * Add exception handling and retry mechanism.
    * Retry with exponential backoff with random jitter to prevent "Thundering herd problem".

📁 invenio-rdm-records (16.4.2 -> 16.6.1 🌈)

    📦 release: v16.6.1
    Revert "bug: add custom comment notification for record/draft requests"
    📦 release: v16.6.0
    bug: add custom comment notification for record/draft requests
    affiliations: update AffiliationsField value
    fix: use uow not self.uow
    serializers: bibtex: Conference paper not falling back to proceedings
    serializers: bibtex: Conference proceeding to proceedings
    serializers: bibtex: year and month using publication date
    📦 release: v16.4.2
    rights: fix serialize condition for controlled license
    rights: fix serialize condition for controlled license
    release: v16.5.1
    optional-doi: fix validation check when user needs a DOI
    pids: add manage permission to be able to skip doi transitions checks
    release: v16.5.0
    doi: handle UI for optional DOI feature

📁 invenio-theme (3.5.1 -> 3.5.2 🐛)

    release: v3.5.2
    form: some UI fixed for the deposit form

📁 invenio-userprofiles (3.0.1 -> 3.0.2 🐛)

    release: v3.0.2
    fixed 404 issue when pressing cancel

📁 invenio-vocabularies (6.9.0 -> 6.11.0 🌈)

    release: v6.11.0
    names: allow to add the acronym
    release: v6.10.1
    names: drop unique constraint on internal id
    release: v6.10.0
    names: add internal id to identify the names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmorrell tmorrell tmorrell left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@karkraeg @tmorrell @wgresshoff