🐛 Moved access to an API property

[ErisDS]

This fix has a fairly small surface area

closes #11574

• the current implementation of the access property has it frontend only, and wired up only in one place
• this leaves it only available in a handful of places, e.g. can't use it in a post loop or get helper
• the current implementation also fails logically if the html content of the post is blank

This fix moves the behaviour to the API

• this ensures the field is always available no matter what context you are in
• it also updates the logic to use the same membersHasAccess logic as is used to gate the post, so it's always correct

TODO: should reconsider the location of this code

@naz @allouis tagging you on this mostly so that you see it go by, but I have one small question - do you thing it makes sense to keep the addition of the access property here in content-gating, or move it to the extra-attrs util? I can see it both ways, here at least all members behaviour is together, but I am adding a prop. If I moved it to the extra-attrs util, I'd have to call checkPostAccess a second time, which seems unnecessary.

[naz]

@ErisDS, not not a super strong opinion but here's my thinking.

I'd prefer putting access field assignment into the extra-attrs util. An extra call to membersHasAccess isn't expensive in terms of speed so calling it second time shouldn't be a big deal. Having new property assignments bundled up in one place has a benefit of not having to remember looking around in different modules when doing an API review. Also, when deciding where to add new property, putting this assignment in different module would raise a question for a person doing a change about why it's separate, even though there's no reason behind it.
TLDR: putting it in extra-attrs - gives less of a headache for future maintenance.

[allouis]

Personally I think it's better to group code by "what it's to do with" rather than "what it does" it lends itself better to more service oriented architecture - which will be useful when splitting out the codebase in the future.

Having new property assignments bundled up in one place has a benefit of not having to remember looking around in different modules when doing an API review.

This is true, but I'd say is more of an issue with how the API is structured, there are a lot of implicit function calls (serialisers getting called because they match the method name of the api controller) and indirection (almost every serilialiser ends up calling some "mapper" module which does exactly what the serialiser should do)

If we look in this mapper module it become clear what we need to check for an API review, and if that was directly linked to the api controller code - it would be even clearer!

That all said, this is a small, easily changeable patch, and I'm not too fussed either way ☺️

[ErisDS]

Looking at this again with both perspectives

I completely understand the point about maintainability, but I think duplicating the requires for both the memberService and labs decreases maintainability more than the extra attribute being added in the content gating specific file.

I think extra-attrs should probably be broken down into something more service or feature-oriented.

I'm gonna leave as-is for now because reducing coupling is a primary goal atm. Have fixed the broken tests