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 publish service to handle non-200 success status codes #797

Merged
merged 2 commits into from
Jan 2, 2025

Conversation

piemonkey
Copy link
Contributor

Overview

When testing calls to prepublish service, I noticed that one of the calls was returning a 201 created status code, which was being interpreted as a failure. I changed this call and some others I found to use the Response.ok field instead of checking for 200. I also replaced the confusing JSON.stringify(error) call as an error always serialises as {}.

connected issues and PRs:

Found when creating lblod/notulen-prepublish-service#124

Setup

This was found when working with an in-progress prepublisher which returned a 201 in place of a 200 for a job status. So, to test, you can run app-gn with a local version of the prepublisher. Modify the /prepublish/job-result/:jobUuid route to return a 201 status on success.

How to test/reproduce

This was visible when creating a job on the endpoint /meeting-notes-previews, so when producing the publish preview for a notulen.

Challenges/uncertainties

N/A

Checks PR readiness

  • UI: works on smaller screen sizes
  • UI: feedback for any loading/error states
  • Check cancel/go-back flows
  • Check database state correct when deleting/updating (especially regarding relationships)
  • changelog
  • npm lint
  • no new deprecations

@elpoelma elpoelma self-requested a review January 2, 2025 15:26
@elpoelma elpoelma force-pushed the fix/handle-non-200 branch from 7e85128 to 4166e84 Compare January 2, 2025 15:34
@elpoelma elpoelma enabled auto-merge January 2, 2025 15:36
@elpoelma elpoelma merged commit 42874e3 into master Jan 2, 2025
2 of 3 checks passed
@elpoelma elpoelma deleted the fix/handle-non-200 branch January 2, 2025 15:38
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.

2 participants