Skip to content

Conversation

@kshepherd
Copy link
Member

@kshepherd kshepherd commented Sep 18, 2025

References

Fixes #2742 and #2572

Description

Fixes an issue with submission section form validation, where fields that are both "required" but also hidden by type-bind (for the current submission based on dc.type) are still validated by the Angular required validator and causing the section to be marked as not valid / display the warning symbol, even though it is successfully validated by the server and can be deposited.

The fix for this is to disable the fields as well as simply hiding them, so they are skipped by the Angular validation process.

To reproduce the problem:

  1. Configure a form (describe) section with a field such as:
        <field>
          <dc-schema>dc</dc-schema>
          <dc-element>contributor</dc-element>
          <dc-qualifier>author</dc-qualifier>
          <label>Author</label>
          <input-type>onebox</input-type>
          <repeatable>false</repeatable>
          <required>You must enter at least the author.</required>
          <hint>Enter the names ...</hint>
          <type-bind>article</type-bind>
        </field>

(modify type-bind value to suit your own value pair or vocabulary for dc.type, or just set dc.type to onebox for testing)

Make sure you have some other fields in the same form, preferably one or more required fields that are NOT type-bound so you can test normal data entry.

  1. Confirm that any submission without the type 'article' does NOT see that field
  2. Note that even when all other required fields in the same section are completed, and there are no server-side errors to display, the section is still marked with an orange "Warning" icon.
  3. Switch to the 'article' type, complete the remaining required fields, and note that the section should now finally be green/success.

Changes

  1. Return MATCHER_ENABLED (inclusive) relation in getTypeBindRelations in addition to MATCHER_VISIBLE, which means that fields not allowed by type bind will be both disabled and hidden
  2. Consolidate duplicate code for getTypeBindRelations into a utility function (in a separate file to avoid circular dependencies) and update the inline documentation

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!

  • My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using npm run lint
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • If my PR fixes an issue ticket, I've linked them together.

@kshepherd kshepherd changed the title Tlc 1202 submission validation matchers main Fix type bind submission form validation handling Sep 18, 2025
@kshepherd kshepherd added port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release port to dspace-9_x This PR needs to be ported to `dspace-9_x` branch for next bug-fix release bug component: submission quick win Pull request is small in size & should be easy to review and/or merge labels Sep 18, 2025
@kshepherd kshepherd moved this to 🙋 Needs Reviewers Assigned in DSpace Maintenance (9.x, 8.x, 7.6.x) Sep 18, 2025
@kshepherd kshepherd force-pushed the TLC-1202_submission_validation_matchers-main branch from 42e3bf3 to 7cfc736 Compare September 18, 2025 14:04
@kshepherd kshepherd force-pushed the TLC-1202_submission_validation_matchers-main branch from 426eb2f to 9244c24 Compare September 18, 2025 14:15
@z-stoynova
Copy link

Our data model requires type-bind for many fields in the locally defined submission forms. Before this fix, Angular validation did not work properly for blocks where the same field was configured multiple times with different properties for different types. This fix completely resolved the issue, and Angular validation now behaves as expected. We are currently running DSpace-CRIS version 2024.02.01. Thank you @kshepherd.

@kshepherd kshepherd changed the title Fix type bind submission form validation handling Fix type-bind submission form validation handling Sep 19, 2025
@tdonohue tdonohue moved this from 🙋 Needs Reviewers Assigned to 👍 Reviewer Approved in DSpace Maintenance (9.x, 8.x, 7.6.x) Sep 22, 2025
@tdonohue tdonohue requested review from atarix83 and nwoodward October 2, 2025 14:28
@tdonohue tdonohue added the 1 APPROVAL pull request only requires a single approval to merge label Oct 2, 2025
@tdonohue tdonohue moved this to 👀 Under Review in DSpace 10.0 Release Oct 2, 2025
Copy link
Contributor

@nwoodward nwoodward left a comment

Choose a reason for hiding this comment

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

👍 Thanks @kshepherd. This PR fixes the errors and warnings with type-bind fields in the submission form for me.

@github-project-automation github-project-automation bot moved this from 👀 Under Review to 👍 Reviewer Approved in DSpace 10.0 Release Oct 8, 2025
@kshepherd kshepherd merged commit 4c0c537 into DSpace:main Oct 9, 2025
15 checks passed
@github-project-automation github-project-automation bot moved this from 👍 Reviewer Approved to ✅ Done in DSpace Maintenance (9.x, 8.x, 7.6.x) Oct 9, 2025
@github-project-automation github-project-automation bot moved this from 👍 Reviewer Approved to ✅ Done in DSpace 10.0 Release Oct 9, 2025
@dspace-bot
Copy link
Contributor

Backport failed for dspace-7_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-7_x
git worktree add -d .worktree/backport-4725-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-4725-to-dspace-7_x
git switch --create backport-4725-to-dspace-7_x
git cherry-pick -x c47d988bca3d0542a7632570c0bdef3dbf6b7ca3 9244c24d118f3a6af1d30ee97f15f6ee43909650 f16dda82554f3d256df3d042ff2134cf6999b112

@dspace-bot
Copy link
Contributor

Successfully created backport PR for dspace-8_x:

@dspace-bot
Copy link
Contributor

Successfully created backport PR for dspace-9_x:

@tdonohue tdonohue removed port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release port to dspace-9_x This PR needs to be ported to `dspace-9_x` branch for next bug-fix release labels Oct 9, 2025
@tdonohue
Copy link
Member

tdonohue commented Oct 9, 2025

@kshepherd : If this should be ported to 7.x, then it looks like we'll have to do that manually. The automatic backport failed

@tdonohue tdonohue added this to the 10.0 milestone Oct 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 APPROVAL pull request only requires a single approval to merge bug component: submission port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release quick win Pull request is small in size & should be easy to review and/or merge

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

DSpace 7: submission forms issue with check for mandatory fields

5 participants