Skip to content

Conversation

geolessel
Copy link
Contributor

@geolessel geolessel commented Sep 16, 2025

This fixes two separate but coupled issues I found while using the package. It started by me not being able to create related resources through the spec-approved path of :parent_type/:parent_id/relationships/:relation_type with a payload of {"data":[{"id":"ID","type":"RELATION"}]}. It appears that while the code converts the keys to atoms, it was actually checking for the string version of the key. Furthermore, any request containing a meta tag failed.

While creating the tests that verified the actual fix for that issue, another issue presented itself: when creating a related resource, the server responded with the correct type but the wrong id. My first thought was it was returning the join table id of the related resource but I actually now think it was returning the id of the parent. Either way, I also fixed the issue and it now returns the correct type/id combination.

While I was at it, I also added explicit tests for the PATCH and DELETE relationships flow as well. While it passes the tests as written, I am unsure about what actually the DELETE response body should contain. Based on my reading of the spec1, I think the body should be empty. And I'm not sure it is expected to return the remaining related resources. I'm looking for your direction here, for sure. Based on a re-read of the correct section of the spec2, I think what we have in here is correct.

Disclaimer: AI was used to track down, fix, and test these bugs. As is usually the case, a lot of steering had to be done to get it to a spot where I was comfortable opening a PR with the code it helped produce. I tried to follow other patterns found in this repo but feel free to massage it (or ask me to) to get it more in line.

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

Footnotes

  1. https://jsonapi.org/format/#crud-deleting-responses-200

  2. https://jsonapi.org/format/#crud-updating-relationship-responses-200

This fixes two separate but couple issues I found while using the
package. It started by me not being able to create related resources
through the spec-approved path of /:parent_id/relationships/:relation
with a payload of `{"data":[{"id":"ID","type":"RELATION"}]}`. It appears
that while the code converts the keys to atoms, it was actually checking
for the string version of the key. Furthermore, any request containing
a `meta` tag failed.

While creating the tests that verified the actual fix for that issue,
another issue presented itself: when creating a related resource, the
server responded with the correct `type` but the wrong `id`. My first
thought was it was returning the _join table_ id of the related resource
but I actually now think it was returning the id of the parent. Either
way, I also fixed the issue and it now returns the correct `type`/`id`
combination.
@zachdaniel zachdaniel merged commit c85bc2e into ash-project:main Sep 18, 2025
21 of 22 checks passed
@zachdaniel
Copy link
Contributor

🚀 Thank you for your contribution! 🚀

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