Skip to content

fix(resources): restore v4 super.post create on collection posts - #1956

Merged
kriszyp merged 11 commits into
mainfrom
fix/instance-post-create
Jul 29, 2026
Merged

fix(resources): restore v4 super.post create on collection posts#1956
kriszyp merged 11 commits into
mainfrom
fix/instance-post-create

Conversation

@ldt1996

@ldt1996 ldt1996 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Restores v4-style instance post overrides that delegate to super.post on a bare collection path. The compatibility normalization runs before authorization and dispatch so the bare and slash forms both use collection/insert semantics, while default bare POST retains its 404 and loadAsInstance=false resources retain their distinct base-resource dispatch. Includes regressions for target parsing, default behavior, insert-vs-update authorization, and false-mode Resource/Collection separation.

Generated with GPT-5.6 Codex.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Resource class to support v4-style instance posting on collection targets when no ID is present, and adds comprehensive unit tests for this behavior and RequestTarget collection semantics. The feedback advises updating the new test assertions to use assert.strictEqual instead of loose assert.equal to align with the repository's style guide.

Comment thread unitTests/resources/crud.test.js Outdated
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

@ldt1996

ldt1996 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

unit failures are pre existing on main

@ldt1996
ldt1996 force-pushed the fix/instance-post-create branch from 89e0899 to 959724c Compare July 27, 2026 06:45

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I think #1807 had this mostly covered, but the override case was missing, thank you for tracking down.

Comment thread resources/Resource.ts Outdated
Comment thread resources/Resource.ts Outdated
@ldt1996
ldt1996 force-pushed the fix/instance-post-create branch from bc1df4d to 0cc84e9 Compare July 28, 2026 19:02
Comment thread unitTests/resources/permissions.test.js Outdated

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I guess GPT thinks this would be a lot safer to restrict this to this.loadAsInstance !== false.
🤖 Reviewed with GPT 5.6

Comment thread resources/Resource.ts Outdated
@kriszyp
kriszyp dismissed stale reviews from themself July 29, 2026 23:57

Addressed in 49a9319 with the loadAsInstance false guard and dispatch regression.

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review threads addressed.

Comment thread unitTests/resources/permissions.test.js Outdated

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aligned with #1842: instance-loaded legacy bare POSTs normalize before authorization, while loadAsInstance=false custom handlers retain their existing Resource/Collection dispatch boundary. All review threads are addressed and CI is green.

Comment thread resources/Resource.ts Outdated
@kriszyp
kriszyp merged commit b04af4d into main Jul 29, 2026
51 of 52 checks passed
@kriszyp
kriszyp deleted the fix/instance-post-create branch July 29, 2026 23:58
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