Skip to content

MIR-1410 show metadata when document is blocked #1087

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 3 commits into
base: 2023.06.x
Choose a base branch
from

Conversation

Antoniafriedrich
Copy link
Contributor

@Antoniafriedrich Antoniafriedrich marked this pull request as draft November 19, 2024 13:30
@Antoniafriedrich Antoniafriedrich marked this pull request as ready for review November 28, 2024 09:27
@Antoniafriedrich Antoniafriedrich force-pushed the MIR-1410-show-metadata-when-document-is-blocked branch from 41b89a3 to f925f03 Compare November 28, 2024 09:28
</xsl:call-template>
<xsl:apply-templates select="mods:name" mode="printName"/>
</xsl:if>
<xsl:if test="$msg">
Copy link
Member

Choose a reason for hiding this comment

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

the $msg is already displayed in line 91

@Antoniafriedrich Antoniafriedrich changed the base branch from 2023.06.x to revert-995-issues/MIR-1303-specific-pdf-does-not-let-metadata-page-render January 20, 2025 11:56
@Antoniafriedrich Antoniafriedrich changed the base branch from revert-995-issues/MIR-1303-specific-pdf-does-not-let-metadata-page-render to 2023.06.x January 20, 2025 12:10
@Antoniafriedrich Antoniafriedrich requested a review from kkrebs March 11, 2025 14:02
@yagee-de yagee-de self-requested a review June 24, 2025 12:15
@kkrebs kkrebs removed request for kkrebs and sebhofmann June 24, 2025 12:17
@kkrebs
Copy link
Contributor

kkrebs commented Jun 24, 2025

looks good for me, but @yagee-de should have a final look at this

@toKrause toKrause changed the title MIR 1410 show metadata when document is blocked MIR-1410 show metadata when document is blocked Jun 24, 2025
@yagee-de yagee-de requested a review from Copilot July 8, 2025 12:17
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new metadata display for blocked documents by introducing a renderMetadata template, refactoring name-printing into a shared helper, and wiring a new MIR.Blocked.Detailpage flag.

  • Added helperTemplates.xsl with printAuthorName for reuse in CSV export, email events, and metadata.
  • Implemented renderMetadata in mods-metadata.xsl and conditionally invoke it when MIR.Blocked.Detailpage is true.
  • Updated mods2csv.xsl, e-mail-events.xsl, and mods-metadata.xsl to include the shared helper and remove duplicated name-printing templates; added a new property in mycore.properties.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
xsl/mods2csv.xsl Included name helper and replaced apply-templates with call-template
xsl/metadata/mods-metadata.xsl Added MIR.Blocked.Detailpage param, conditional block, renderMetadata, and helper include
xsl/import/helperTemplates.xsl New file defining shared printAuthorName template
xsl/e-mail-events.xsl Included name helper and replaced apply-templates with call-template
config/mir/mycore.properties Added default MIR.Blocked.Detailpage=false setting
Comments suppressed due to low confidence (4)

mir-module/src/main/resources/xsl/import/helperTemplates.xsl:6

  • [nitpick] Template name 'printAuthorName' may be misleading since it's used to print names in various contexts (e.g., CSV exports and email events). Consider renaming to a more generic name like 'printPersonName' or 'printName'.
    <xsl:template name="printAuthorName">

mir-module/src/main/resources/xsl/metadata/mods-metadata.xsl:95

  • [nitpick] The new conditional block and renderMetadata template are not covered by existing tests. Consider adding unit or integration tests to verify metadata rendering when MIR.Blocked.Detailpage is true.
                <xsl:if test="$MIR.Blocked.Detailpage = 'true'">

mir-module/src/main/resources/config/mir/mycore.properties:769

  • [nitpick] This new property controls whether metadata is displayed on blocked detail pages. Consider adding a comment above to document its purpose and acceptable values.
MIR.Blocked.Detailpage=false

mir-module/src/main/resources/xsl/metadata/mods-metadata.xsl:102

  • This call to printAuthorName immediately after renderMetadata appears to duplicate author output outside of the metadata block. Consider removing or integrating author printing within renderMetadata to avoid duplication.
                    <xsl:call-template name="printAuthorName">

@@ -182,4 +198,72 @@
</a>
</xsl:template>

<xsl:template name="renderMetadata">
Copy link
Preview

Copilot AI Jul 8, 2025

Choose a reason for hiding this comment

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

[nitpick] The renderMetadata template is fairly large and includes hard-coded HTML structure. Consider breaking it into smaller sub-templates or adding comments to improve readability and maintainability.

Copilot uses AI. Check for mistakes.

@rsteph-de
Copy link
Member

Please fix idention

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.

4 participants