Skip to content

Conversation

@tw4l
Copy link
Member

@tw4l tw4l commented Dec 15, 2025

Fixes #3065

This PR modifies the crawlconfig PATCH endpoint to do what myself and most users probably always assumed it did, which is to only modify fields in the CrawlConfig.config that are explicitly included in the PATCH request, rather than replacing the entirety of the config with what's in the PATCH request.

Arguably this could be seen as a breaking change, but I think given the semantics of PATCH and the fact that at least some users were confused by the previous behavior, I think we might be okay to just consider this a bugfix without need for a compatibility flag or similar in the API endpoint.

Tests have been updated accordingly.

Manual testing steps

  1. Spin up a local Browsertrix instance off of main
  2. Create and save a workflow with some config fields filled out - e.g. set browser language to English and a custom user agent
  3. Update local Browsertrix instance to this branch
  4. Submit a PATCH request with cURL or similar for with a payload containing 1-2 config field updates (e.g. {"config": {"blockAds": true}}) and verify that the rest of the config remains as it was originally saved.
  5. Submit a PATCH request with some config fields in the payload but using their current values, verify that the response includes "settings_changed":false

tw4l added 2 commits December 15, 2025 17:18
This is a bit more complex than checking if the other crawlconfig
attributes have changed, especially for seeds.
@tw4l tw4l requested a review from ikreymer December 15, 2025 22:28
@tw4l tw4l changed the title Issue 3065 crawlconfig patch Only modify CrawlConfig.config fields included in PATCH request Dec 15, 2025
Copy link
Member

@ikreymer ikreymer left a comment

Choose a reason for hiding this comment

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

Thanks for the thorough tests! Tweaked a bit to have a simplified version with exclude_unset=true, also updated returns true only if actually updated.

@ikreymer ikreymer merged commit 2793d2c into main Dec 17, 2025
24 checks passed
@ikreymer ikreymer deleted the issue-3065-crawlconfig-patch branch December 17, 2025 20:53
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.

[Bug]: PATCH crawlconfig with partial config update causes data loss

3 participants