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.

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

Save records on creation/duplication and allow attachments #1107

Merged
merged 10 commits into from
Feb 10, 2025

Conversation

cmoinier
Copy link
Collaborator

@cmoinier cmoinier commented Jan 31, 2025

First of!

This PR is a joint work of the record creation & duplication tickets, to allow saving the record and adding attachments in both cases.
A huge thanks to @Angi-Kinas @tkohr @ronitjadhav for the joint work on this PR!

Description

This PR makes sure the attachments are available upon record creation & duplication
To this aim, all newly created/duplicated records are now saved but not published. They can be manually published even without any changes done to the new draft.
A new Observable manages that, named savedButNotPublished which carries the information given by the backend for each record, from a search request.
The column "Status" of the dashboard is now also based on the response from the search request, and displays the right info.
It is now impossible in all cases to delete from the draft table (since all drafts have a record).
Multiple UT & e2e have been reworked and/or deleted, since the "draft only" status does not exist anymore.

Architectural changes

  • All mentions & uses of alreadySavedOnce are removed because all records are now saved once. An alternative, if needed, if to use savedButNotPublished but no component needs it for now.
  • For the same reasons, all mentions & uses of "isDraftOnly" have been removed. In some cases, for safety (because the backend takes a few seconds to save the record), they have been replaced with a check that a uuid exists for the record or not. I'm not even sure that this safety measure is necessary.
  • Also did a big update on the italian translations because the CI run was long 😄

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

@cmoinier cmoinier force-pushed the duplicate-attachments branch from 88cb3f8 to 5e88452 Compare January 31, 2025 10:44
Copy link
Contributor

github-actions bot commented Jan 31, 2025

Affected libs: api-repository,
Affected apps: metadata-editor,

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

Copy link
Contributor

github-actions bot commented Jan 31, 2025

📷 Screenshots are here!

@cmoinier cmoinier force-pushed the duplicate-attachments branch 2 times, most recently from 6b27bdf to 3739173 Compare February 5, 2025 11:17
@coveralls
Copy link

coveralls commented Feb 5, 2025

Coverage Status

coverage: 83.982% (+0.01%) from 83.972%
when pulling 958b57b on duplicate-attachments
into 9ee0df3 on main.

@cmoinier cmoinier force-pushed the duplicate-attachments branch 3 times, most recently from d561f17 to 4a313ad Compare February 6, 2025 11:14
@cmoinier cmoinier changed the title [wip] Create with attachments Save records on creation and allow attachments Feb 6, 2025
@cmoinier cmoinier force-pushed the duplicate-attachments branch from 4a313ad to d3cd4bd Compare February 6, 2025 11:20
@cmoinier cmoinier changed the title Save records on creation and allow attachments Save records on creation/duplication and allow attachments Feb 7, 2025
Copy link
Collaborator

@jahow jahow left a comment

Choose a reason for hiding this comment

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

Tested, works really well!

I just noticed a slight glitch when creating a record:

Peek 07-02-2025 11-08

the editor form jumps down and back up quickly; the reason is unclear but it was quite noticeable.

Thank you all for your work!

@cmoinier
Copy link
Collaborator Author

cmoinier commented Feb 7, 2025

Tested, works really well!

I just noticed a slight glitch when creating a record:

Peek 07-02-2025 11-08 Peek 07-02-2025 11-08

the editor form jumps down and back up quickly; the reason is unclear but it was quite noticeable.

Thank you all for your work!

Thanks for catching that :) It was because the "/create" page was loaded for a very short time, because "/edit" was not ready yet. I have added a loading spinner to display the "/edit" page only when it has a record ready, it should be smoother.

Copy link
Collaborator

@Angi-Kinas Angi-Kinas left a comment

Choose a reason for hiding this comment

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

Thanks @cmoinier for this huge feature! It was a lot of work and you completed it! 🎉

The code looks good, I just have a few comments and questions. I did not test the feature yet though. Let me know if something is not clear.

Thanks again for your hard work on this!

@cmoinier cmoinier force-pushed the duplicate-attachments branch from b7b3fe4 to 04bd352 Compare February 10, 2025 09:51
@cmoinier cmoinier force-pushed the duplicate-attachments branch from f9d84b9 to e1f5ed4 Compare February 10, 2025 10:59
@cmoinier cmoinier force-pushed the duplicate-attachments branch from e1f5ed4 to 958b57b Compare February 10, 2025 11:01
@cmoinier cmoinier merged commit 2804667 into main Feb 10, 2025
14 checks passed
@cmoinier cmoinier deleted the duplicate-attachments branch February 10, 2025 11:33
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.

5 participants