NYS 168: Restrict MCP block content edit access to blocks linked to their managed senators

[nathanielwoodland]

Note that the related hotfix in #226 revokes all MCP block content access. This is the long term fix.

[nathanielwoodland]

@routinet look I used UsersHelper for this! Glad you kept putting that in my face, since I've found there are helper methods in there that work very well for this (and make a lot of sense for NYS's internal logic).

But also ... don't look, since I didn't wrap ->getStorage('node') in a try/catch block again. Since our last chat, I've found a lot of code in core (and contrib) that calls ->getStorage('node')--or the storage ID of other core Drupal entity types--without any extra exception handling. Given how frequently this method will be called in custom code, I'd love to simplify our usage of ->getStorage() to only worry about exception handling for non-core entity types. Let me know what you think. Of course, I'm susceptible to arm twisting on this one (especially when the person doing the arm twisting is the tech lead on the project ....). :)

[routinet]

getStorage() can throw an exception. try..catch pls

[nathanielwoodland]

@routinet, just making sure you saw my comment from 2 weeks ago about the omitted try/catch. My instinct is to follow Drupal core's coding standards regarding getStorage() called on core entity types, but if you think the try/catch is still worthwhile, I propose adding it without actually logging anything (to avoid adding a dependency on the logger factory whenever we access node storage). Lmk if you're okay with either A. full omission of try/catch for core types, or B. try/catch without logging. Ty!

[routinet]

including it without logging is fine. I'm just concerned about whatever weird edge case is going to pop up and trigger an uncaught exception. I dislike it also, but... whachagonnado? (also dislike a do-nothing catch, but I feel it is the lesser of two evils)

[nathanielwoodland]

Great, all set @routinet