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

GN-4362 fix signatures disappearing after publishing #499

Merged
merged 3 commits into from
Jun 28, 2023

Conversation

abeforgit
Copy link
Member

@abeforgit abeforgit commented Jun 22, 2023

Overview

For notulen, the VersionedNotulen you sign is not the same as the one
you publish. So I had to revert my earlier change to go back to saving
any non-empty signedresource relationships.

connected issues and PRs:

https://binnenland.atlassian.net/browse/GN-4362

Setup

How to test/reproduce

See ticket, make meeting, sign, then publish before, signature
dissapeared now, it doesnt

Challenges/uncertainties

Checks PR readiness

  • UI: works on smaller screen sizes
  • UI: feedback for any loading/error states
  • Check cancel/go-back flows
  • Check database state correct when deleting/updating (especially
    regarding relationships)
  • changelog
  • npm lint
  • no new deprecations

For notulen, the VersionedNotulen you sign is not the same as the one
you publish. So I had to revert my earlier change to go back to saving
any non-empty signedresource relationships.
@abeforgit abeforgit added the bug Something isn't working label Jun 22, 2023
@abeforgit abeforgit requested review from dkozickis and x-m-el June 22, 2023 08:32
Copy link
Contributor

@x-m-el x-m-el left a comment

Choose a reason for hiding this comment

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

It works as expected 👍

I can't really grasp the code though. Is this correct:

  • first there is no versionedNotulen. a dummy versionedNotulen record is created to show instead.
  • a user can then sign, which will create a signed resource (and also a versionedNotulen in the backend? What happens for a second signature?)
  • Can publish, which also creates a versionedNotulen (with a publishedResource attached).

In loadNotulen:

  1. if published, that notulen is always used
  2. if no published but a signed notulen, that signed notulen is used
    Is there always only one signed notulen (with one or two signatures)?
  3. if no signing/publish, a "dummy" notulen record is used

I think notulenSet would better be named publishedNotulenSet to make it clear why it shouldn't be set to true when set for a signedResource.

@abeforgit
Copy link
Member Author

It works as expected +1

I can't really grasp the code though. Is this correct:

* first there is no versionedNotulen. a dummy versionedNotulen record is created to show instead.

* a user can then sign, which will create a signed resource (and also a versionedNotulen in the backend? What happens for a second signature?)

* Can publish, which also creates a versionedNotulen (with a publishedResource attached).

In loadNotulen:

1. if published, that notulen is always used

2. if no published but a signed notulen, that signed notulen is used
   Is there always only one signed notulen (with one or two signatures)?

3. if no signing/publish, a "dummy" notulen record is used

I think notulenSet would better be named publishedNotulenSet to make it clear why it shouldn't be set to true when set for a signedResource.

sounds about right but indeed the logic is completely bonkers
not gonna refactor it now though not enough time

@abeforgit abeforgit enabled auto-merge June 28, 2023 14:48
@abeforgit abeforgit merged commit c833571 into master Jun 28, 2023
@abeforgit abeforgit deleted the GN-4362-signatures-disappear-after-publishing branch June 28, 2023 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants