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

hotfix: remove empty decisions from document on save #393

Merged
merged 4 commits into from
Dec 21, 2022
Merged

Conversation

usrtim
Copy link
Contributor

@usrtim usrtim commented Dec 20, 2022

In order to prevent users from being able to create “faulty” documents by copy pasting etc, we would remove decisions that don’t have any content from the document when saving.

@usrtim usrtim requested review from abeforgit and elpoelma December 20, 2022 15:20
const body = parsedHtml.body.childNodes;

for (const child of body) {
if (child.attributes?.typeof?.textContent.includes('besluit:Besluit')) {
Copy link
Member

Choose a reason for hiding this comment

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

I would check for both the prefixed and full URI here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nvdk please let me know if new commit address your concern ?

Copy link
Contributor

@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.

Works as expected. When removing all content of a decision (but keeping its div) it is correctly removed.

Copy link
Member

@abeforgit abeforgit left a comment

Choose a reason for hiding this comment

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

This only checks the top level children of the document. What if the div in question is further down the tree?
As we mentioned in the standup, there is https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelectorAll which will probably be useful here

@elpoelma elpoelma requested a review from abeforgit December 21, 2022 16:55
@abeforgit abeforgit merged commit c80445d into master Dec 21, 2022
@abeforgit abeforgit deleted the fix/GN-3907 branch December 21, 2022 21:57
@abeforgit abeforgit added the bug Something isn't working label Dec 21, 2022
abeforgit pushed a commit that referenced this pull request Dec 21, 2022
* hotfix: remove empty decisions from document on save

* add check for both the prefixed and full URI

* use querySelectorAll in removeEmptyDivs

Co-authored-by: Elena Poelman <[email protected]>
Co-authored-by: Arne Bertrand <[email protected]>
(cherry picked from commit c80445d)
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.

4 participants