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

[BUGFIX] Fix publishbackgroundall dependency index #121

Conversation

dreistromlandMf
Copy link
Contributor

The command in2publish:workflow:publishbackground from the premium plugin may skip pages depending on previous pages:

  1. Assume a page and a child page are scheduled for publishing.
  2. Assume the child page gets processed first.
  3. The child page will fetch the parent page as a dependency. The parent page will get recorded in the index.
  4. When the parent page record gets fetched from the database, it will already be in the index. It will not get added to any record index recording.
  5. The RecordTreeBuilder assumes that pages scheduled for publishing get added to the record index recording buildRecordTree, which did not happen in step 4.
  6. The parent page gets marked as published, but changes will not actually have been published.

This commit ensures the parent page gets added to the buildRecordTree record index recording. It archives that by adding pages to the record index recording even if it was already in the index.

As this code is pretty deep, I am not quite sure about unintended side effects. Intense testing is advisable.

The command in2publish:workflow:publishbackground from the premium
plugin may skip pages depending on previous pages:

 1. Assume a page and a child page are scheduled for publishing.
 2. Assume the child page gets processed first.
 3. The child page will fetch the parent page as a dependency. The
    parent page will get recorded in the index.
 4. When the parent page record gets fetched from the database, it will
    already be in the index. It will not get added to any record index
    recording.
 5. The RecordTreeBuilder assumes that pages scheduled for publishing
    get added to the record index recording buildRecordTree, which did
    not happen in step 4.
 6. The parent page gets marked as published, but changes will not
    actually have been published.

This commit ensures the parent page gets added to the buildRecordTree
record index recording. It archives that by adding pages to the record
index recording even if it was already in the index.

As this code is pretty deep, I am not quite sure about unintended side
effects. Intense testing is advisable.
@sbusemann
Copy link
Contributor

@sbusemann sbusemann added this to the in planning milestone Oct 28, 2024
@tinzog tinzog added in doing and removed planned labels Nov 4, 2024
@tinzog tinzog self-assigned this Nov 4, 2024
@tinzog
Copy link
Contributor

tinzog commented Nov 4, 2024

Thank you very much for supplying this helpful PR.
We are currently testing for unintended side effects.

@dreistromlandMf dreistromlandMf marked this pull request as ready for review November 4, 2024 14:51
@tinzog
Copy link
Contributor

tinzog commented Nov 6, 2024

We could reproduce the problem and tested the PR successfully. Thanks for your investigation and support.

@tinzog tinzog merged commit 164ee06 into in2code-de:develop Nov 6, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants