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.

Sign up for GitHub

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

Jump to bottom

[ARP - lower camel responses - 5] bypass olive branch #20625

Merged
merged 2 commits into from
Feb 7, 2025
Merged

[ARP - lower camel responses - 5] bypass olive branch #20625

merged 2 commits into from
Feb 7, 2025

Conversation

nihil2501
Copy link
Contributor

@nihil2501 nihil2501 commented Feb 5, 2025 •
edited
Loading

See head of PR stack: #20621

olive_branch transforms (a) request and (b) response payloads.

(a) It deeply transforms request and query param keys.
At times it is convenient for params to act as snake-cased setters at a Ruby interface, but not always. Form resources are a possible example where not.

For now, let's wait to encounter the cases where we really want this convenience. If we do encounter some, we may discover that we want a more explicit and collocated way to opt in.

(b) It reloads the response from JSON, deeply transforms keys, and dumps back to JSON.
This is superfluous because our serialization layer jsonapi-serializer already has a configuration option for key casing. This realizes our desired casing the one and only time it is visiting an object during serialization.

@nihil2501 nihil2501 changed the title Art/lower camel responses/5 [ARP - lower camel responses - 5] bypass olive branch Feb 5, 2025
@va-vfs-bot va-vfs-bot temporarily deployed to art/lower-camel-responses/5/main/main February 5, 2025 17:11 Inactive
@nihil2501 nihil2501 mentioned this pull request Feb 5, 2025
Merged
@nihil2501 nihil2501 force-pushed the art/lower-camel-responses/4 branch from 57de93d to bc28d66 Compare February 6, 2025 00:57
@nihil2501 nihil2501 force-pushed the art/lower-camel-responses/5 branch from 568ad0a to 22bb41e Compare February 6, 2025 00:58
@va-vfs-bot va-vfs-bot temporarily deployed to art/lower-camel-responses/5/main/main February 6, 2025 00:59 Inactive
@nihil2501 nihil2501 marked this pull request as ready for review February 7, 2025 22:33
@nihil2501 nihil2501 requested review from a team as code owners February 7, 2025 22:33
@nihil2501 nihil2501 merged commit 3380b21 into art/lower-camel-responses/4 Feb 7, 2025
39 checks passed
@nihil2501 nihil2501 deleted the art/lower-camel-responses/5 branch February 7, 2025 22:36
nihil2501 added a commit that referenced this pull request Feb 7, 2025
@nihil2501
[ARP - lower camel responses - 4] camel response in ARP users control…
7350c54 
…ler (#20624)

* [ARP] camel response in ARP users controller

* [ARP - lower camel responses - 5] bypass olive branch (#20625)

* [ARP] bypass olive branch

* lint fixes
nihil2501 added a commit that referenced this pull request Feb 7, 2025
@nihil2501
[ARP - lower camel responses - 3] handle camel params in ARP IPF cont…
f544157 
…roller (#20623)

* [ARP] handle camel params in ARP IPF controller

* [ARP - lower camel responses - 4] camel response in ARP users controller (#20624)

* [ARP] camel response in ARP users controller

* [ARP - lower camel responses - 5] bypass olive branch (#20625)

* [ARP] bypass olive branch

* lint fixes
nihil2501 added a commit that referenced this pull request Feb 7, 2025
@nihil2501
[ARP - lower camel responses - 2] camel case POA form persistence (#2…
16d1459 
…0622)

* [ARP] camel case POA form persistence

* lint fixes

* [ARP - lower camel responses - 3] handle camel params in ARP IPF controller (#20623)

* [ARP] handle camel params in ARP IPF controller

* [ARP - lower camel responses - 4] camel response in ARP users controller (#20624)

* [ARP] camel response in ARP users controller

* [ARP - lower camel responses - 5] bypass olive branch (#20625)

* [ARP] bypass olive branch

* lint fixes
nihil2501 added a commit that referenced this pull request Feb 11, 2025
@nihil2501
[ARP - lower camel responses - 1] set_key_transform :camel_lower (#…
adf715c 
…20621)

* [ARP] `set_key_transform :camel_lower`

* lint fixes

* [ARP - lower camel responses - 2] camel case POA form persistence (#20622)

* [ARP] camel case POA form persistence

* lint fixes

* [ARP - lower camel responses - 3] handle camel params in ARP IPF controller (#20623)

* [ARP] handle camel params in ARP IPF controller

* [ARP - lower camel responses - 4] camel response in ARP users controller (#20624)

* [ARP] camel response in ARP users controller

* [ARP - lower camel responses - 5] bypass olive branch (#20625)

* [ARP] bypass olive branch

* lint fixes
ojbucao pushed a commit that referenced this pull request Feb 11, 2025
@nihil2501 @ojbucao
[ARP - lower camel responses - 1] set_key_transform :camel_lower (#…
a115a22 
…20621)

* [ARP] `set_key_transform :camel_lower`

* lint fixes

* [ARP - lower camel responses - 2] camel case POA form persistence (#20622)

* [ARP] camel case POA form persistence

* lint fixes

* [ARP - lower camel responses - 3] handle camel params in ARP IPF controller (#20623)

* [ARP] handle camel params in ARP IPF controller

* [ARP - lower camel responses - 4] camel response in ARP users controller (#20624)

* [ARP] camel response in ARP users controller

* [ARP - lower camel responses - 5] bypass olive branch (#20625)

* [ARP] bypass olive branch

* lint fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chumpy chumpy chumpy approved these changes

@pixiitech pixiitech pixiitech approved these changes

@ojbucao ojbucao Awaiting requested review from ojbucao

Assignees
No one assigned
Labels
benefits-accredited-rep-facing require-backend-approval test-passing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nihil2501 @chumpy @pixiitech @va-vfs-bot