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

fix(docs): don't merge payloads for PUT requests #740

Merged
merged 3 commits into from
Jan 31, 2023

Conversation

kanadgupta
Copy link
Member

@kanadgupta kanadgupta commented Jan 31, 2023

🚥 Fixes #738

🧰 Changes

For some background, here's the general workflow for when syncing a Markdown file to ReadMe via the rdme docs (or changelogs/custompages) command:

  1. Grab the slug from the page
  2. Make a GET request to the ReadMe API to confirm that the page exists
  3. If the page exists, update the page via a PUT request
  4. If the page does not exist, create the page via a POST request

The problem in step 3 is that we take the response body from the GET request (called existingDoc in the code below) and merge that with the Markdown file's data when making the PUT request:

body: JSON.stringify(
Object.assign(existingDoc, {
...payload,
})
),

This isn't really necessary since the API already merges the PUT request body with the existing doc, and it's creating adverse side effects when our API expects one field and not another (see #738 for an example).

🧬 QA & Testing

Do tests pass?

@kanadgupta kanadgupta added bug Something isn't working command:docs Issues pertaining to the `docs`, `changelogs`, or `custompages` commands labels Jan 31, 2023
@kanadgupta kanadgupta requested a review from erunion January 31, 2023 21:05
@kanadgupta kanadgupta marked this pull request as ready for review January 31, 2023 21:05
Copy link
Contributor

@garrett-wade garrett-wade left a comment

Choose a reason for hiding this comment

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

Looks good, had the exact same code in a branch ready to go :)

@kanadgupta kanadgupta merged commit e3c7f22 into main Jan 31, 2023
@kanadgupta kanadgupta deleted the fix/dont-merge-payloads branch January 31, 2023 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working command:docs Issues pertaining to the `docs`, `changelogs`, or `custompages` commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when updating doc using docs command with categorySlug and parentSlug
3 participants