Skip to content

Change how renderer output is converted to JSON #133

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

Merged
merged 4 commits into from
Oct 5, 2021

Conversation

malcolmbaig
Copy link
Contributor

Context

  • The 0.4.0 release has a bug which incorrectly serialises part of the document, returning arrays of over-escaped strings rather than arrays of JSON hashes in the data field.
  • This bug was fixed in master.
  • The fix introduced another bug which leads to inconsistencies in the output JSON. This PR attempts to fix that bug.

Changes proposed in this pull request

Update the railtie to fix issues with the way renderer output is converted into a JSON string in the controller.

Cached and non-cached resources need slightly different handling, so make a logical check to tell the difference and use the appropriate method.

malcolmbaig and others added 4 commits September 24, 2021 06:42
Update the railtie to fix issues with the way renderer output is
converted into a JSON string in the controller.

Cached and non-cached resources need slightly different handling, so
make a logical check to tell the difference and use the appropriate
method.
These specs test the caching version of the render call more thoroughly.

- Assert that the expected data structure is stored in the cache
- Assert that the response is the same whether using the caching version
  of this call or the regular one
- Git ignore tmp cache files
- Add pry-byebug to dev dependencies
Change how renderer output is converted to JSON
@remear
Copy link
Member

remear commented Oct 5, 2021

Thanks for this!

@remear remear merged commit 163f371 into jsonapi-rb:master Oct 5, 2021
@phanle
Copy link

phanle commented Feb 14, 2022

@remear Can we release a new version of the gem to have this commit?

@remear
Copy link
Member

remear commented Feb 14, 2022

My intent was to first fix the CI to ensure a stable release, of which there's a PR up I need to go review, then get this shipped. Sadly, I haven't had as much time to work on this as I'd like. I'll try to wrangle some time to get this out soon.

@remear remear mentioned this pull request Feb 24, 2022
@jkva
Copy link

jkva commented Mar 8, 2022

Seconding this - I'm dealing with the same JSON serialisation double encoding problem since I've tried to add caching, would love to see this released 😁

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.

4 participants