fix: error 431 when having a lot of relationship items
Description
To fix the issue of GET request getting a too long query variable as described in #6486, I have implemented X-HTTP-Method-Override header to allow this query to be sent as a POST request and giving the information that is it actually a GET request.
- [X] I have read and understand the CONTRIBUTING.md document in this repository.
Type of change
- [X] Bug fix (non-breaking change which fixes an issue)
Checklist:
- [X] I have added tests that prove my fix is effective or that my feature works
- [X] Existing test suite passes locally with my changes
@andershermansen would you mind creating a test that tests for this?
It would need to be done under test/fields-relationship/e2e.spec.ts. I think it would be relatively simple.
Generate data with SDK
- Use the
payloadSDK client present in that file to create many items for a relatable collection, i.e. manyrelation-onedocs - Then use the SDK to create a doc for the
fields-relationshipcollection and set the all the ID's (except for 1, save it for later!) created in step 1 to therelationshipHasManyfield within it.
Use Playwright to test
- Then use Playwright to attempt to add that remaining relationship doc
- Ensure that the doc can be added successfully
The test should fail without your change and pass with it. If you can do this that would be very helpful! Let me know what you think.
@JarrodMFlesch Thanks for the description on how to do the test. It helped a lot. This is my first time to write a playwright test, so there was some tweaking. Please review and let me know if something needs to be improved.
The test was very useful as it also got another case when I increased the amount of items even more (not_in is longer than in, so with even more items one more place failed).
Running the test on previous branch shows the failure, and on my branch it shows the success.
Here is the failure on beta branch, it times out waiting for the options which does not show up because of the API error:
@JarrodMFlesch A friendly reminder about this fix. If you want the fix or the test done in a different way, let me know and I can work on it.
@DanRibbens Thanks. I have created and pushed docs as well.
@andershermansen I am ready to merge this in! The only change I would request is the docs example. I think most of our users will feel more comfortable with a fetch example rather than an http example. Can you make that slight adjustment?
I see you already adjusted it. Good change.
@andershermansen I did, I want to merge it in and close your issue 👼
Hi, quick question, is this also planned to be fixed in version 2. ?