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

Fix versionedNotulen never fetching filecontent #118

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

Conversation

abeforgit
Copy link
Member

@abeforgit abeforgit commented Nov 14, 2024

This pr does 2 related things:
The first commit is the actual bugfix: we were checking for the existence of a file in the wrong way, causing it to always generate a false negative, creating a new file instead of using the old one.

Then a different part of the code was getting the content, and it only allowed there to be a single file. This is a good check, but the error it generated dissapeared and never made it to the frontend, so we couldn't handle it if we tried.

For this I adjusted our usage of Tasks to be compliant with the model

needs auth config from lblod/app-gelinkt-notuleren#213

jira: GN-5226

@abeforgit abeforgit changed the title fix(signing): correctly check for file content Fix second signature Nov 14, 2024
@abeforgit abeforgit changed the title Fix second signature Fix versionedNotulen never fetching filecontent Nov 15, 2024
Copy link
Collaborator

@elpoelma elpoelma left a comment

Choose a reason for hiding this comment

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

The re-use of the notulen file seems to work well now for the second signature, confirmed by debugging :)
I did not test the error producing/handling yet, any tips on how to do this best?

I did notice an additional error (unrelated to this PR): lezer mock-users are not able to provide the first signature, this is not expected right?
Update: this is probably caused due to the inability to make files as a 'lezer'. Only accounts with the 'Schrijver' role may create files (https://github.com/lblod/app-gelinkt-notuleren/blob/587962bf82a7cd5c47ccafd7826fb37405b03680/config/authorization/config.ex#L154C26-L154C97)

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.

2 participants