feat!: Make load/create/update/delete entity authorization result interfaces consistent

[wschurman]

Why

The idea here is to unify the pattern that we use for loading across mutations as well.

Currently, loading looks like:

await TestEntity.loader(vc).enforcing().loadByIdAsync();
await TestEntity.loader(vc).withAuthorizationResults().loadByIdAsync();

But mutations still look like:

await TestEntity.creator(vc).setField(...).enforceCreateAsync();
await TestEntity.creator(vc).setField(...).createAsync();

await TestEntity.updater(entity).setField(...).enforceUpdateAsync();
await TestEntity.updater(entity).setField(...).updateAsync();

await TestEntity.enforceDeleteAsync(entity);
await TestEntity.deleteAsync(entity);

This is proving to be too error-prone. Mostly with the deleteAsync/enforceDeleteAsync distinction not being explicit enough since the returned result from deleteAsync is rarely used in application code.

This PR proposes changing the mutations to be more like the loading:

await TestEntity.creator(vc).enforcing().setField(...).createAsync();
await TestEntity.creator(vc).withAuthorizationResults().setField(...).createAsync();

await TestEntity.updater(entity).enforcing().setField(...).updateAsync();
await TestEntity.updater(entity).withAuthorizationResults().setField(...).updateAsync();

await TestEntity.deleter(entity).enforcing().deleteAsync();
await TestEntity.deleter(entity).withAuthorizationResults().deleteAsync();

How

Add layer in-between entity methods and mutator factories that requires dictating enforcing or withAuthorizationResults to vend a mutator.

Note that this will likely require a codemod in any application that uses it. But the codemod is pretty straightforward.

Test Plan

This has full test coverage.

[wschurman]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• chore: add CI check for needing to run ctix #258
• feat!: Default all loaders and mutators to enforcing #256
• feat!: Add enforcing entity association loader #257
• chore: switch barreling to ctix #255
• chore: internal refactor of loader to be structured like new mutation methods #254
• feat!: Make load/create/update/delete entity authorization result interfaces consistent #253 👈 (View in Graphite)
• chore: upgrade typescript and related packages #252
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (8286d43) to head (a9b658b).

Additional details and impacted files

@@                                       Coverage Diff                                        @@
##           @wschurman/02-19-chore_upgrade_typescript_and_related_packages      #253   +/-   ##
================================================================================================
  Coverage                                                          100.00%   100.00%           
================================================================================================
  Files                                                                  72        78    +6     
  Lines                                                                1985      2033   +48     
  Branches                                                              279       278    -1     
================================================================================================
+ Hits                                                                 1985      2033   +48     

Flag	Coverage Δ
integration	100.00% <100.00%> (ø)
unittest	100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ide]

Looks fine but I think it'd be a good idea to push out a breaking change to Entity where everything is enforcing by default and you need to call unenforcing() (or withAuthorizationResults()). We talked about this before I believe. Almost all our callsites use enforcing() and we'd be making the common case the default.

[wschurman]

Yep. This change to making it explicit should help with the proposed breaking change that changes the default (assuming upgrades are done one-by-one version). In the current state though (before this PR) we can't do the breaking change without creating risk of implicit return type changes which are harder to detect (wouldn't necessarily show up for tsc run); for this package, we should strive to make all breaking changes detectable via tsc.