Regenerate PDF when secretary changes document
Opened this issue · 7 comments
Currently we only do manual regenerations. This was not know by the secretary, resulting in a ton of outdated PDFs. There's a call in topdesk to regenerate those that still can be regenerated (M240300564)
However, we should add a regenerate-step to the change documents page ASAP to avoid this for the near future.
Long term, we should change the way we link to documents from the PDF to not directly link to the files themselves, but to a 'document resolves' page that never changes (even if the underlying document does).
Long term, we should change the way we link to documents from the PDF to not directly link to the files themselves, but to a 'document resolves' page that never changes (even if the underlying document does).
Absolutely.
Though I thought that as long as a proposal is being reviewed the PDF is always regenerated? use_canonical_pdf should always be false when changes can still be made. Though I'm saying that without looking at the code.
I didn't look too closely at the code, so that might be the intended behavior. But I can 100% confirm that that is not always the case. (I had to manually regenerate a couple of them).
So, if that's indeed the case we have to debug that as well.
So, I think I know what happened.
The canonical-PDF-protection kicks in as soon as status_review is not none. Which is indeed what Michael described.
So, I think this is a order-of-user-operations problem. The code expects the following:
- All decisions are made
- Secretary updates any documents
- Secretary closes review <-- PDF is frozen
- Secretary sends confirmation letter
However, I think Desiree actually does the following:
- All decisions are made
- Secretary closes the review <-- PDF is frozen
- Secretary updates any documents
- Secretary sends confirmation letter
Which, tbh, from a workflow point-of-view isn't actually all that strange. So, if this indeed the case, I kinda think we don't need to do any further changes. My hotfix allows the secretary to update documents at any time, which solves the problem in all cases. It's somewhat dangerous tho, as the protection is there for a reason.
So, alternatively we could move the protection to step 4, confirmation letter sending. We do have a field for that in the DB, date_confirmed, so we could modify the check to include a date_confirmed is not None.
Eh, no, that alternative also does not work. There's no confirmation letter when status_review == False, but that is still a valid 'lock PDF' condition. So, (status_review == True and date_confirmed is not None) or status_review == False?
Perhaps that could just be (date_confirmed is not None) or (status_review is False) ? Although I guess it doesn't matter all that much now that the PDF is regenerated when documents change.
And yeah, you hit the nail on the head with your diagnosis of the problem.
Yes that would be functionally the same. I do prefer the more verbose one as that highlights the 'status_review is None' case. However, that could also be a comment, in which case your version is better.
Although I guess it doesn't matter all that much now that the PDF is regenerated when documents change.
True, but I think highlighting this case here is a good idea as it can avoid confusion on what should be allowed.
In addition, if we change the conditions like this, we can remove the forced-update I think? We'd still have to clearly indicate that the PDF is locked when the letter is send, but that way we are sure we don't have accidental updates later on.
Een status update van deze issue (ik zag dat er een vraag was in je notulen van vorige week).
Het probleem is in principe gefixt. Deze staat echter nog open omdat er mogelijk een betere fix is (die ook andere, tot nu toe theoretische, randgevallen zou opvangen).
Het is iig geen priority meer, ik degradeer hem dus ff in de prioritering.