Commit 77528e1
authored
LEGLINK-828: Tenant service integration with the DMRP module (#1848)
* LEGLINK-828: Derive facility scheduled reports from DMRP reporting plans
The Tenant facility endpoints now resolve their state-changing operations through
IFacilityOperations, which the DMRP module replaces with its own implementation
when DMRP:Enabled is set. Routes and the BFF reverse proxy are unchanged - only
the behavior behind POST, PUT and DELETE differs.
With DMRP enabled:
- Create and update refuse a caller-supplied scheduledReports block and derive
the schedule from the facility's reporting plans instead, grouping the mapped
dQMs by the frequency their measure mapping carries. A measure with no dQM
mapped is logged and excluded rather than silently dropped.
- Hard delete removes the facility first, then its reporting plans, so plans
survive a delete the host refuses. Soft delete keeps them, since the facility
can be restored.
- The reporting period is read in the facility's own timezone, so a facility near
a month boundary is scheduled against the month it is actually in.
Plans are read from the FacilityReportingPlans table behind IReportingPlanSource.
An implementation that refreshes those rows from the DMRP API takes its place with
LEGLINK-698/701, and nothing consuming the interface changes.
The module takes over the IFacilityOperations registration and delegates to the
host's implementation, which it names through a type parameter, so a host that
does not supply one fails to compile rather than at startup.
Also adds a facility lifecycle test asserting Quartz job presence across create,
soft delete, restore and delete. The existing controller tests assert status codes
only, so scheduling could be dropped from any of those paths unnoticed.
* LEGLINK-828: Point the scheduled reports refusal at a remedy that works
End-to-end testing showed the refusal named something the caller cannot do.
TenantScheduledReportConfig's Daily, Weekly and Monthly are non-nullable, so a
request that leaves scheduledReports out is rejected during model binding with
"The Daily field is required" before the DMRP module runs. Telling the caller to
resubmit without the block sent them to a different 400 with no way forward.
An empty block is what gets through, so the message asks for that instead.
Adds tests pinning both halves of the contract: an explicitly empty block is
accepted rather than treated as a caller-supplied schedule, and the refusal names
the empty-array remedy. Neither is reachable from a unit test of the operations
alone - the binding failure sits above where they call in - so the message could
drift back without the second one.
* TECH_DEBT: Document the DMRP controller endpoints
Both DMRP controllers carried a one-line summary per action and nothing else, so
the parameter ranges, the validation rules and the meaning of each status code
lived only in the code. All fifteen endpoints now carry param and response
documentation, which Swagger picks up through the DMRP XML file the Tenant
service already feeds it. The same text is on the matching requests in the Link
Admin BFF Postman collection.
Behavior that was previously undocumented and is easy to get wrong:
- Reporting plans refuse out-of-range paging rather than clamping it, while
measure mappings quietly replace it with the default. The two controllers
disagree, so both now say which they do.
- An empty measure mapping search answers 204, while an empty reporting plan
search answers 200 with an empty page.
- A measure mapping created without a frequency defaults to Adhoc, which
schedules nothing.
- Deleting a measure mapping that reporting plans still reference answers 404
rather than a conflict, because the manager reports a refused delete and a
missing row as the same failure. Distinguishable only by the message.
Also drops the 404 ProducesResponseType from DeleteAllMeasureMappings, which has
no path that returns one.
* LEGLINK-828: Let the Admin UI create a facility when DMRP is enabled
With DMRP enabled the Tenant API derives a facility's scheduled reports from its
DMRP reporting plans and refuses a request that supplies its own. The facility
form did the opposite: ScheduledReportsValidator required at least one report, so
no input satisfied both and facility create and edit were impossible. The local
docker stack has been in that state since DMRP__Enabled was set true there; no
deployed environment is affected, because DMRP:Enabled is absent from every store
and defaults to false.
The form now asks the flag what to do:
- ScheduledReportsValidator takes it and drops the "at least one report" rule when
DMRP is on. Duplicates are still rejected either way.
- The report pickers are replaced by a line saying the schedule is derived.
- Submit sends empty arrays. Editing an existing facility loads its stored
schedule into the controls, so they are emptied explicitly rather than assumed
untouched. The block itself is still sent: its arrays are not nullable, so
omitting it fails model binding before the API sees it.
DMRP:Enabled moves from the Tenant service section of the config catalog to
global. It is one decision for the deployment rather than one service's setting,
and unlabeled means every service reads the same answer. The Angular app cannot
read App Configuration, so the same decision reaches it through the container's
LINK_DMRP_ENABLED. The two must agree, and the failure modes are not symmetric:
the UI switched on ahead of the services quietly creates facilities that report
nothing, while the reverse fails loudly. The catalog and the code both say so.
The flag is temporary and expected to end up permanently on, so it is written to
be deleted: every site is marked "DMRP feature flag", the removal recipe is on
AppConfig.dmrpEnabled, and the validator parameter defaults to the state the flag
settles on, with a test pinning that.
* LEGLINK-828: Refuse a referenced measure mapping with a conflict, not a not-found
Deleting a measure mapping that facility reporting plans still reference answered
404. The row plainly exists and is readable, so reporting it as missing tells the
caller the opposite of what happened. The manager wrapped every save failure in
ApplicationException, which the controller could only map one way.
The mapping is now asked about before it is deleted: if any reporting plan
references it, the request is refused with 409 and a message naming the reason.
Delete-all is refused the same way while any plan exists, checked before anything
is removed so a refusal deletes nothing rather than part of the table. A mapping
that genuinely does not exist still answers 404.
The check is a query rather than a translated database error because the error is
not stable enough to classify on. Which one the database raises depends on EF's
change tracker: with the dependent untracked the DELETE reaches the database and
the foreign key fires (SQL Server 547, SQLite 787), but with it tracked EF first
tries to sever the relationship by nulling MeasureMappingId, which the NOT NULL
column rejects instead (SQL Server 515, SQLite 1299). Classifying on the code
alone would have behaved differently in a controller request than in a test. The
translation is kept as a backstop for the window between the check and the delete,
now covering all four codes.
Also adds ValidateAntiForgeryOrBearerToken to DeleteMeasureMapping, which was the
only write endpoint on the controller without it.
Both defects were found while documenting these endpoints and were called out then
as belonging to the measure mappings work rather than the DMRP facility
integration.
* LEGLINK-828: Address review findings and unblock the Backend E2E suite
Log arguments in the two new DMRP files called Sanitize(), the HTML sanitizer.
It strips markup but leaves CR/LF, so a facilityId or measure containing a
newline could still forge log lines. Both values come from request bodies. They
now call SanitizeForLog(), which replaces control characters, matching every
sibling in the module. Five call sites across DbBackedReportingPlanSource and
DmrpFacilityOperations.
The DeleteAllAsync pre-check added earlier refuses while any reporting plan
exists, which turned a leaked row into an unrelated failing test: the three DMRP
integration classes share one SQLite file, run in an unspecified order, and
cleared plans in their constructor only. Cleanup now also runs in Dispose, so a
class cannot leak rows into whichever runs next.
Adds the two tests the new branches were missing:
- DeleteAllMeasureMappings refused with 409 while a reporting plan exists, also
asserting an unreferenced mapping survives the refusal.
- The facility form with DMRP enabled, covering the case the conditional exists
for: editing a facility loads its stored schedule into the report controls, so
submitting an otherwise untouched facility has to send empty arrays rather than
hand the stored schedule back to an API that refuses it.
Turns DMRP off in the docker stack. FacilitySetupHelper creates facilities with a
schedule of its own, which the refusal rejects, so enabling DMRP there broke the
Backend E2E suite. DMRP:Enabled in appsettings.Docker.json and LINK_DMRP_ENABLED
in docker-compose move to false together - the UI switched on ahead of the
services is the silent failure mode, so they are never flipped separately. The
compose comment records what to set to exercise DMRP locally.
* LEGLINK-828: Delete a facility and its reporting plans in one transaction
Hard delete removed the facility, then its reporting plans, with nothing tying
the two together. A failure after the facility row was gone stranded the plans:
nothing ever collected them, they blocked measure mapping deletes, and a facility
later created with the same id silently inherited a previous incarnation's
schedule.
Both persist through the host's database context - the facility repository and
the reporting plan repository resolve the same scoped instance - so one
transaction covers them. The order is unchanged, since the host's delete can
still refuse and plans removed ahead of a refused delete would leave a facility
that reports nothing.
Quartz keeps its own store and cannot enlist, so a rollback leaves the restored
facility without its scheduled jobs. That was the argument against a transaction
here, and it does not hold: ScheduleService.StartAsync rebuilds jobs for every
facility that is not deleted, and DeleteJobsForFacility is idempotent, so the
gap closes on the next restart or delete retry. A stranded reporting plan closes
on nothing.
The rollback is guarded. If it fails it is logged with the facility and the
endpoint that clears its plans, and the original exception is rethrown - the
caller needs to hear about the delete that failed, not the cleanup that failed
afterwards.
Tests cover the transaction protocol in order, rollback without commit when the
plan cleanup throws, and the original exception surviving a failing rollback.
* LEGLINK-828: Cover the measure mapping delete backstop and refuse a mis-ordered module registration
- Add unit coverage for MeasureMappingManager's IsStillReferenced backstop, which
handles the window between the pre-check and the delete. A SQLite foreign key
failure (787) and a not-null failure (1299) both surface as
MeasureMappingInUseException, a failure buried two levels deep still does, and an
unrelated failure does not. SQL Server's 547 and 515 cannot be reached from a
test: SqlException has no public constructor and is only ever produced by the
driver.
- Refuse AddDmrpModule when the host has registered neither IFacilityOperations nor
its implementation type. RemoveAll on a service nobody registered removes nothing
and reports nothing, so calling the module before the host registered its
operations left the host's registration appended afterwards and winning the
resolve: DMRP enabled with none of its facility behavior and no sign of it.
- Give the missing-dQM exclusion test a frequency the schedule has a bucket for, so
the dQM filter is what excludes the entry rather than the frequency mismatch, and
cover the adhoc frequency in a test of its own.
- Drop a single-use intermediate list when logging unmapped measures.
* LEGLINK-828: Set up automation facilities correctly whether or not DMRP is enabled
The automation stack posted a facility's scheduled reports with the facility, which
DMRP refuses, so every Automation.UI scenario run failed at setup with the module
enabled. The Backend E2E suites are thin clients of Automation.UI's run API, so they
failed for the same reason and in the same place.
- Detect DMRP by asking rather than by configuration. A disabled module strips its own
controllers from the host, so api/dmrp/reporting-plans answers 404 when it is off and
200 when it is on. An answer that is neither stops the run naming the request that
failed, rather than guessing and stranding it later.
- With DMRP off, post the schedule with the facility exactly as before. With it on,
create the facility with an empty schedule, enroll it in a measure mapping per
measure, then save it again so Tenant derives the schedule from those reporting
plans. Neither ordering works on its own: the schedule is derived when the facility
is saved, but a reporting plan is refused for a facility that does not exist yet.
- Map each measure to itself monthly. The run drives the pipeline with the measure's
own id, so the derived schedule names what the report types name and both paths leave
the same facility behind. That is what keeps the tenant database validator unchanged.
- Enroll for the following reporting period as well as the current one, so a run that
crosses midnight on the first of a month does not derive an empty schedule from a
period nothing was enrolled for.
- Reuse a measure mapping that already exists, including one created concurrently.
Mappings are shared by every run against a stack, so creating one is a race a run can
lose without having failed.
- Fix SearchMeasureMappingsAsync, which read the measure-mapping collection route. That
route only accepts POST, so the call answered 404 - indistinguishable from the module
being switched off - and it had no consumers to notice. Add the measure, dQM and
frequency filters the API already supports.
- Correct the DMRP switch comment in docker-compose.yml: the E2E suite no longer forces
the module off, and appsettings is baked into the tenant image, so changing the flag
needs a rebuild rather than just recreating the container.
* LEGLINK-828: Cover the DMRP endpoints in API Health
The DMRP module's endpoints were the only service surface API Health did not exercise,
so the behaviours added for this story - a conflict when deleting a referenced measure
mapping, a refused facility schedule, no-content on an empty search - sat outside the
suite CI runs.
- Add a DMRP suite covering measure mapping and reporting plan CRUD, their error paths,
and the one endpoint DMRP changes without owning: a facility that carries its own
schedule is refused while the module is enabled.
- Ask the Tenant service whether DMRP is registered rather than reading a flag. A
disabled module strips its own controllers, so its routes answer 404, and the suite
reports its steps as skipped rather than failing a stack that simply is not running
DMRP. This keeps the switch in one place instead of adding a copy here.
- Declare no seed requirement. Seeding runs a full pipeline scenario before any suite
executes, which is a minute or two of waiting that buys this suite nothing and is
wasted entirely whenever DMRP is switched off. The two fixtures it needs are cheap to
obtain directly: a dQM MeasureEval already holds, and a facility of its own that it
removes when it is done. Running the suite drops from about two minutes to one second.
- Stop counting skipped steps against a service on the API Health dashboard. The badge
showed passed-over-total with skipped steps in the denominator and the same grey as a
run in progress, so a service whose steps all skipped read as one that was still
going. Skipped steps now leave the denominator and a fully skipped service says so.
Steps that never ran stay in it, so pending and skipped no longer look alike.
* LEGLINK-828: Stop the facility form requiring at least one scheduled report
The rule predated DMRP and is removed at a reviewer's suggestion on this PR. It also could
not coexist with DMRP: the schedule is derived from a facility's reporting plans and the
Tenant API refuses a facility that supplies its own, so with the rule in force there was no
input the form could produce that the API would accept. The DMRP work routed around it by
making the rule conditional; removing it outright takes the contradiction out at the source.
- ScheduledReportsValidator now checks one thing, that no report is named twice, and no
longer takes the DMRP flag. Its errors accumulator went with the second rule.
- Drop the noReportsEntered getter and the error it rendered on the facility form.
- Cover a duplicate repeated within a single period, which the spec had not exercised.
The DMRP flag stays in the UI: it still hides the report pickers and submits an empty
schedule, the latter mattering on edit, where the form loads a facility's stored schedule
into the controls and would otherwise send it back to an API that refuses it. Its
retirement recipe loses the step for the validator parameter.
Note this also applies with DMRP disabled, where the form was the only thing preventing a
facility from being saved with no scheduled reports. The API has always allowed it -
ScheduleService treats an empty array as "create no job for this frequency" - so such a
facility can now be created from the UI and will run no scheduled reports.1 parent 909a7fb commit 77528e1
44 files changed
Lines changed: 3687 additions & 160 deletions
File tree
- DotNet
- Automation.Link/Helpers
- Automation.UI
- Services
- ApiHealth/TestSuites
- Views/ApiHealth
- DMRP
- Business
- Managers
- Controllers
- DependencyInjection
- Models/Exceptions
- LinkSdk/Clients
- Interfaces
- ServiceTests
- IntegrationTests
- DMRP
- Tenant
- UnitTests
- Automation
- DMRP
- LinkSdk
- Tenant
- Business
- Controllers
- Web/Admin.UI
- server
- src
- app
- components
- tenant/facility-config-form
- validators
- services
- assets
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
| 7 | + | |
6 | 8 | | |
7 | 9 | | |
8 | 10 | | |
| |||
13 | 15 | | |
14 | 16 | | |
15 | 17 | | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
16 | 24 | | |
17 | 25 | | |
| 26 | + | |
18 | 27 | | |
19 | 28 | | |
20 | | - | |
| 29 | + | |
| 30 | + | |
21 | 31 | | |
22 | | - | |
23 | | - | |
| 32 | + | |
| 33 | + | |
24 | 34 | | |
25 | 35 | | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
26 | 48 | | |
27 | 49 | | |
| 50 | + | |
28 | 51 | | |
29 | 52 | | |
30 | | - | |
| 53 | + | |
| 54 | + | |
31 | 55 | | |
32 | | - | |
| 56 | + | |
33 | 57 | | |
34 | 58 | | |
35 | 59 | | |
36 | | - | |
| 60 | + | |
37 | 61 | | |
38 | 62 | | |
39 | 63 | | |
| 64 | + | |
| 65 | + | |
40 | 66 | | |
41 | 67 | | |
42 | 68 | | |
43 | 69 | | |
44 | | - | |
| 70 | + | |
45 | 71 | | |
46 | 72 | | |
47 | 73 | | |
48 | 74 | | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
56 | 79 | | |
57 | 80 | | |
58 | 81 | | |
59 | 82 | | |
60 | 83 | | |
61 | 84 | | |
62 | 85 | | |
63 | | - | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
64 | 315 | | |
65 | 316 | | |
66 | 317 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
251 | 251 | | |
252 | 252 | | |
253 | 253 | | |
| 254 | + | |
254 | 255 | | |
255 | 256 | | |
256 | 257 | | |
| |||
0 commit comments