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

Editor: Fix error when creating new record #1061

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Editor: Fix error when creating new record #1061

wants to merge 3 commits into from
+210 −193

Conversation

tkohr
Copy link
Collaborator

@tkohr tkohr commented Dec 11, 2024 •
edited
Loading

Description

The goal of this PR is to fix the error when creating a new record. The same exception sometimes occurs when editing fields of a draft. A first commit ensures errors displayed via the notifications.service still to be logged to the console.

The actual error is caused by the fact that the EditRecordResolver is called even when routing to /create. This happens due to the navigate['edit'] calls within the component. Both navigations have been removed:

  • 07dcd58 does not seem essential, but maybe I've missed something. Creating a new record now stays on /create. => This prevents the exception when creating a new record.
  • a97b566 does not seem to be used anymore, since we've replaced the uniqueIdentifier with the resourceIdentifier in the form. => This prevents the exception when editing fields of a draft.

Note to reproduce: The issue occurs on the first access to /create after app reload. It starts with the second access to /create or accessing /create after /edit.

Screenshots

image

Quality Assurance Checklist

  • Commit history is devoid of any merge commits and readable to facilitate reviews
  • If new logic ⚙️ is introduced: unit tests were added
  • If new user stories 🤏 are introduced: E2E tests were added
  • If new UI components 🕹️ are introduced: corresponding stories in Storybook were created
  • If breaking changes 🪚 are introduced: add the breaking change label
  • If bugs 🐞 are fixed: add the backport <release branch> label
  • The documentation website 📚 has received the love it deserves

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 11, 2024 •
edited
Loading

Affected libs: feature-editor, feature-notifications, feature-search, feature-router, feature-map, feature-record,
Affected apps: metadata-editor, datafeeder, demo, datahub, webcomponents, map-viewer, search,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 11, 2024 •
edited
Loading

📷 Screenshots are here!

tkohr added 3 commits December 12, 2024 16:20
@tkohr
feat(notifications-service): log errors in console when displaying er…
a94d731 
…ror notifications
@tkohr
fix(edit-page): do not navigate to edit during create
07dcd58 
since this provokes calling the edit-record.resolver which needs a metadata xml too early and results in an exception
note: bug ccould be reproduced from any second access to /create
@tkohr
fix(edit-page): do not navigate to edit with new id when uniqueIdenti…
a97b566 
…fier changes

this causes exceptions when modifying fields in a draft.
it does not seem to be used anymore, since the uniqueIdentifier has been replaced by the resourceIdentifier
@tkohr tkohr marked this pull request as ready for review December 12, 2024 15:33
@coveralls
Copy link

Coverage Status

coverage: 83.672% (+1.9%) from 81.764%
when pulling a97b566 on me-fix-create
into a63d6d0 on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tkohr @coveralls