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

Harden content drafts and force-disable drafts when coupled with workbench_moderation #192

Merged
merged 7 commits into from
Nov 2, 2023

Conversation

stooit
Copy link
Contributor

@stooit stooit commented Oct 31, 2023

  • workbench_moderation is not currently supported and has issues when the "disable drafts" option is not enabled
  • default disable_content_drafts to on
  • harden the use of the quant-token header when content drafts are disabled

@kepol
Copy link
Contributor

kepol commented Oct 31, 2023

I'll test this as well.

@kepol
Copy link
Contributor

kepol commented Oct 31, 2023

I haven't tested fully yet, but here's a scenario that still needs work:

  1. don't install workbench moderation module yet
  2. uncheck "Disable content drafts" in quant settings and save
  3. update quant module so new update hook runs (doesn't do anything because WM module not yet installed)
  4. install workbench moderation module
  5. go to quant settings and it shows "Disable content drafts" defaulted to enabled
  6. don't save the quant settings
  7. value in the database for "Disable content drafts" doesn't match quant settings in UI
  8. uninstall workbench moderation module
  9. you'll see "Disable content drafts" is still unchecked

We need to add hook_modules_installed code to do same thing as update hook.

@kepol
Copy link
Contributor

kepol commented Oct 31, 2023

I can add the hook_modules_installed code now.

@kepol
Copy link
Contributor

kepol commented Oct 31, 2023

Screenshot 2023-11-01 at 10 04 02 AM

Screenshot 2023-11-01 at 10 06 31 AM

Screenshot 2023-11-01 at 10 05 46 AM

@kepol
Copy link
Contributor

kepol commented Oct 31, 2023

Managed to completely bork my test 9.5 site by installing/uninstalling workbench moderation... now content isn't syncing at all :/

I think due to having content moderation enabled first...

@kepol
Copy link
Contributor

kepol commented Oct 31, 2023

Actually, it seems to be when Disable content drafts is enabled that I can't get content to sync when published. I tried with a new test site and both workbench moderation and content moderation modules uninstalled.

$disable_drafts = $config->get('disable_content_drafts');
if (!$disable_drafts) {
$headers['quant-token'] = \Drupal::service('quant.token_manager')->create($route);
}
Copy link
Contributor

@kepol kepol Oct 31, 2023

Choose a reason for hiding this comment

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

With the if (!$disable_drafts) { logic in place, I can't get published content to sync at all when Disable content drafts is enabled. I tested this with both Workbench Moderation and Content Moderation modules uninstalled.

When I removed the check in both places, then it worked as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Latest commit 2bca4a7 will resolve by disabling token verification when drafts are disabled -- glad you picked that up!

@kepol
Copy link
Contributor

kepol commented Nov 1, 2023

The new changes work fine when no moderation modules are enabled. I tested with the following and quant always looked correct. I still need to test with the two moderation modules.

setting unchecked

  1. created unpublished content
  2. published the content
  3. unpublished the content

setting unchecked

  1. created published content
  2. unpublished the content
  3. published the content

setting checked

  1. created unpublished content
  2. published the content
  3. unpublished the content

setting checked

  1. created published content
  2. unpublished the content
  3. published the content

@kepol
Copy link
Contributor

kepol commented Nov 1, 2023

I've tested carefully with the latest code with:

  1. no moderation modules installed
  2. core content moderation module installed (and workbench moderation module uninstalled)
  3. workbench moderation module installed (and core content moderation module uninstalled)

Note there is nothing preventing both from being installed at the same time, but there will be strange behavior and errors.

I went between draft and published in various orders for #1.

I went between draft, published, and archived in various orders for #2.

I went between draft, review, published, and archived in various orders for #3.

Everything worked as expected. Quant published state was always the correct value.

@kepol
Copy link
Contributor

kepol commented Nov 1, 2023

Oh, there are some linting errors I can fix.

@kepol
Copy link
Contributor

kepol commented Nov 2, 2023

I'll get this merged and up to d.o repo.

@kepol kepol merged commit 465f4bc into 8.x-1.x Nov 2, 2023
2 checks passed
@kepol kepol added enhancement New feature or request Drupal 8 / 9 / 10 labels Nov 2, 2023
@kepol kepol deleted the feat/harden-internal-token branch November 5, 2023 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Drupal 8 / 9 / 10 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants