ResourceTestBase::getExpectedUnauthorizedAccessMessage() should be removed or replaced with something smarter

Created on 18 December 2024, 2 days ago

Problem/Motivation

ResourceTestBase wrongfully assumes that if you get a 403 to a resource, the reason will always be the same. This is far from the case, as demonstrated for example in 📌 Introduce AccessResult::andAllowedIf(), orAllowedIf(), andForbiddenIf() and orForbiddenIf() to eventually replace orIf() and andIf() Needs review . Yet getExpectedUnauthorizedAccessMessage() receives nothing but the method to determine the message by.

Steps to reproduce

Look at MR from issue above
See how all failing tests are because ResourceTestBase is making too many assumptions about code it really shouldn't be covering in the first place

Proposed resolution

Well a complete rewrite of all jsonapi tests would be nice so we can finally make caching / performance improvements without constantly having to worry about how our unrelated changes will break jsonapi tests again.

But in absence of such a solution it would be nice to simply drop getExpectedUnauthorizedAccessMessage() or replace it with something that has access to the entire context so that any subclass can properly figure out whether there even is an access reason to begin with and, if so, what that reason is.

Remaining tasks

Decide on how to fix this.

User interface changes

N/A

Introduced terminology

N/A

API changes

TBD

Data model changes

N/A

Release notes snippet

N/A

🐛 Bug report
Status

Active

Version

11.1 🔥

Component

jsonapi.module

Created by

🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Production build 0.71.5 2024