-
Notifications
You must be signed in to change notification settings - Fork 141
refactor(banners): Replace the payment-failure evaluator with a generic backend command intake #4909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
refactor(banners): Replace the payment-failure evaluator with a generic backend command intake #4909
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
0eec509
refactor(banners): Replace the payment-failure evaluator with a comma…
MontaGhanmy 3d4810c
test(banners): fail fixture loading through t instead of panicking
MontaGhanmy 36d95f9
fix(banners): re-localize commanded banners on a language change
MontaGhanmy de627cb
fix(banners): apply both ends of a command's validity window
MontaGhanmy 16cb174
refactor(banners): correct the error comment and the success log
MontaGhanmy 4ab9f1d
docs(banners): correct the retry and dead-letter description
MontaGhanmy 49db1bf
ref(docs): trim banners.md, remove redundant information
MontaGhanmy 861eb4f
fix(banners): target organizations by tenant ID
MontaGhanmy 2328c50
fix(banners): skip instances that disallow command categories
MontaGhanmy 2816d56
fix: update banner documents to retain command state
MontaGhanmy 3a93280
fix: update banner integration tests to use shared setup
MontaGhanmy a0811b1
fix: update banner fanout to continue after instance errors
MontaGhanmy a27af44
fix(banners): warn on skipped instances and allowlist CTA hosts
MontaGhanmy f5cdc7f
fix: update banner test whitespace for lint
MontaGhanmy c31b2b3
fix: update MinIO test image registry
MontaGhanmy 2b20252
refactor(banners): rename the organization target to orgId
MontaGhanmy 3d0c7d6
refactor(banners): group banner settings and allow a category wildcard
MontaGhanmy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. too much text here, and it feels we are just repeating what the ADR said
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trimmed in 49db1bf |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| ## Banners | ||
|
|
||
| This is the publisher reference for backend banner commands. The stack stores | ||
| one `io.cozy.banners` document per category and instance. See | ||
| [ADR 054](https://github.com/linagora/twake-workplace-private/blob/main/documentation/docs/adrs/adr-054.md) | ||
| for the platform design. | ||
|
|
||
| ### Configuration | ||
|
|
||
| Enable banners and allow the publisher's categories in each recipient context: | ||
|
|
||
| ```yaml | ||
| contexts: | ||
| b2b_twake_default: | ||
| banner: | ||
| enabled: true | ||
| command_categories: | ||
| - billing | ||
| - trial | ||
| cta_hosts: | ||
| - manager.example.org | ||
| ``` | ||
|
|
||
| Set `command_categories` to `["*"]` to allow every category except `quota`. | ||
| `cta_hosts` has no wildcard. | ||
|
|
||
| Broker credentials, permissions and bindings control who can publish. Each | ||
| category must have one owner and one addressing mode: the stack keeps one | ||
| revision per instance and category, shared by `orgId` and `workplaceFqdn` | ||
| commands, so never address a category both ways. `quota` is reserved for the | ||
| stack's rules. | ||
|
|
||
| Instances that disable banners are skipped. Instances whose context does not | ||
| list the category or a CTA host are skipped with a warning log. Other eligible | ||
| recipients still receive the command. Skipping an instance leaves its existing | ||
| documents and recorded revision unchanged. An error on one instance | ||
| does not stop processing the others. The stack returns all failures after | ||
| attempting every recipient, so delivery can be retried. | ||
|
|
||
| ### Commands | ||
|
|
||
| Publish JSON on the `platform` exchange, consumed by `stack.banner.commands`. | ||
| The routing key selects the operation: | ||
|
|
||
| - `banner.materialize`: create or replace the banner in a category. | ||
| - `banner.clear`: expire the banner in a category while retaining its revision; nonempty presentation fields | ||
| are rejected. | ||
|
|
||
| See [RabbitMQ configuration](rabbitmq.md#configuration) for queue declarations | ||
| and [shared fixtures](../model/banner/testdata) for complete examples. | ||
|
|
||
| `banner.materialize`: | ||
|
|
||
| ```json | ||
| { | ||
| "workplaceFqdn": "alice.twake.app", | ||
| "eventId": "banner-command-42", | ||
| "revision": 42, | ||
| "timestamp": 1788944400, | ||
| "category": "billing", | ||
| "bannerId": "billing.grace.cycle-a.attempt-2", | ||
| "severity": "warning", | ||
| "surface": "banner", | ||
| "dismissible": true, | ||
| "text": { "en": "We could not charge your card.", "fr": "Nous n'avons pas pu débiter votre carte." }, | ||
| "cta": { | ||
| "label": { "en": "Update payment method", "fr": "Mettre à jour le moyen de paiement" }, | ||
| "url": "https://manager.example.org/linagora/twake_prod/premium" | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| `banner.clear`: | ||
|
|
||
| ```json | ||
| { | ||
| "workplaceFqdn": "alice.twake.app", | ||
| "eventId": "banner-command-43", | ||
| "revision": 43, | ||
| "timestamp": 1788944400, | ||
| "category": "billing" | ||
| } | ||
| ``` | ||
|
|
||
| | Field | Required | Contract | | ||
| | --- | --- | --- | | ||
| | `category` | always | Matches `^[a-z][a-z0-9-]{0,31}$`; `quota` is rejected. | | ||
| | `workplaceFqdn` / `orgId` | exactly one | A single instance host name / a B2B organization ID matching instance `org_id`, whose members receive the command; `orgId` is at most 256 bytes with no surrounding whitespace. | | ||
| | `revision` | always | Positive counter, increasing per category. | | ||
| | `timestamp` | always | Decision time in positive epoch seconds, within the RFC3339 range. Does not order commands. | | ||
| | `eventId` | no | Correlation ID, at most 256 bytes. | | ||
| | `bannerId` | materialize | Matches `^[a-z0-9.-]{1,64}$`. Keep it for the same occurrence to preserve dismissal; change it for a new occurrence. | | ||
| | `severity` | materialize | `info`, `warning` or `error`. | | ||
| | `surface` | materialize | `banner` or `modal`. | | ||
| | `text` | materialize | Locale map with nonempty `en`; at most 1024 bytes per locale. | | ||
| | `title` | no | Locale map with nonempty `en` when supplied; at most 256 bytes per locale. | | ||
| | `cta`, `secondaryCta` | no | Each has a locale-map `label` (nonempty `en`, at most 128 bytes per locale) and an absolute `https` `url` (at most 2048 bytes) whose host is in `banner.cta_hosts`. A secondary CTA requires a primary one. | | ||
| | `dismissible` | no | Defaults to false. A modal without a CTA is made dismissible. | | ||
| | `priority` | no | 0–1000; defaults to 0. Quota banners use 50 and 100. | | ||
| | `startsAt`, `endsAt` | no | RFC3339. If both are supplied, `startsAt` must precede `endsAt`. An explicit start replaces the stored start; omission preserves it for the same occurrence when compatible with the end, otherwise defaults to the command's decision time. | | ||
|
|
||
| Each locale map accepts at most 32 locales with keys of 1–35 bytes. The JSON | ||
| body is limited to 256 KiB, including whitespace and unknown fields. | ||
| `_id`, `_rev`, `dismissedAt` and `cozyMetadata` are not command fields and are | ||
| ignored if supplied. | ||
|
|
||
| ### Localization | ||
|
|
||
| The publisher supplies all wording. The stack selects the instance's locale | ||
| only if it is complete for every supplied text and label; otherwise the whole | ||
| banner falls back to `en`. The stored `lang` identifies the selected language. | ||
| Any complete publisher-supplied locale is supported, independently of the | ||
| stack's translation catalogs. | ||
|
|
||
| On an instance language change, existing banners are re-localized from retained | ||
| commands in the banner documents without republishing. Cleared or deleted | ||
| banners and older records without retained wording are left unchanged. | ||
|
|
||
| ### Revisions and recovery | ||
|
|
||
| Commanded banners store `revision`, `eventId`, and the full localized command | ||
| in `accepted` alongside their presentation. A clear retains the category's | ||
| document with `cleared: true`, an expired `endsAt`, and no retained wording; | ||
| clients must filter out banners whose validity window has ended. A newer | ||
| materialize replaces it normally. Updating the command revision also updates | ||
| the document revision, even when its visible wording is unchanged. | ||
|
|
||
| These fields use the same app permissions as the banner. Apps recording a | ||
| dismissal should preserve the other fields and use the current CouchDB `_rev`; | ||
| editing or deleting the ordering state can allow stale commands to be replayed. | ||
|
|
||
| - Revisions at or below the last accepted revision for an instance and category | ||
| are ignored, even after a clear. Only a changed decision needs a new revision; | ||
| the publisher must ensure newer revisions carry newer state. | ||
| - Retry with the original revision, event ID and payload. Replays complete | ||
| partial organization deliveries and reach newly provisioned members while | ||
| leaving recipients that already accepted the revision unchanged. | ||
| - Enabling banners does not bootstrap them: the publisher must republish. | ||
| - Invalid commands and missing workplaces fail delivery. The broker requeues | ||
| failures without a delay until its configured delivery limit is exhausted; | ||
| configure dead lettering as described in [RabbitMQ](rabbitmq.md#dead-letter-exchange-dlx-and-dead-letter-queue-dlq). | ||
| Fix the cause and explicitly replay dead-lettered commands with their original | ||
| operation routing key (`banner.materialize` or `banner.clear`). | ||
| - The stack sends no application acknowledgement. A broker confirm means the | ||
| broker accepted the message, not that a banner was stored or displayed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you remove the translations here?
The idea is that the stack knows the instance language and it will materialize the banner using that instance language.
we should just keep that system, but the events and calculations are done in cloudery
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stack doesn't choose billing wording anymore, so those entries would never be read again. What we did lose is re-localization, and that's a separate thing: fixed in
36d95f94b, where the private command document keeps every locale the Cloudery sends and the refresh hook picks one again.