Padreug
15c2276e57
Some checks failed
lint.yml / feat(nostr): publish the active ticket wave, not the roll-up (pull_request) Failing after 0s
Since upstream v1.6.8 price, currency and inventory belong to time-boxed ticket waves, and the event-level fields `sync_event_ticket_waves` derives are the PRIMARY wave's price/currency and the SUM of every wave's stock. The NIP-52 publisher read those, so as soon as an organiser created a second wave the public card would advertise the early-bird price after early bird closed and count stock in waves that had not opened. Refs #61. `build_nip52_event` now describes the wave a buyer can actually buy from: - several waves can be open at once, and a publisher has no one to ask which one the buyer wants (the purchase endpoint errors with "Please select a ticket wave"), so it advertises the CHEAPEST open wave — the price a buyer is able to obtain. Deviation recorded in docs/upstream-candidates.md. - with no open wave, `tickets_available` is 0 and never omitted: omission used to mean "unlimited", which #34/#62 removed as a concept. - `tickets_payment_methods` is scoped to the advertised wave too. It was derived from `event.allow_fiat` — the primary wave's — so it could offer a fiat rail while `tickets_allow_fiat` was absent and the purchase endpoint would refuse it. They are the same fact and now come from the same place. Wave boundaries are time-driven, and every republish we have is sale-driven, so nothing fires when early bird ends at midnight. Rather than add a scheduler, a publish records which wave it advertised (`nostr_published_wave_id`, m004) and the reconciliation sweep compares that against the wave that would be advertised now, setting `nostr_publish_pending` on a mismatch — reusing the existing retry path. NULL means "never published", which the sweep leaves alone so an upgrade does not republish the whole table on first boot. The selection rule lives in `models.advertised_ticket_wave` so the publisher and the drift detector cannot disagree about what is on the relay. 17 new tests; 114 pass. ruff, black, prettier clean; mypy error set still identical to HEAD's baseline.
173 lines
7.5 KiB
Markdown
173 lines
7.5 KiB
Markdown
# Rebase playbook
|
|
|
|
How to merge an upstream release into this fork without shipping the
|
|
failures a clean `git merge` cannot see.
|
|
|
|
Written during the v1.6.1 → v1.6.8 rebase (#33) after the first two
|
|
instances showed up within minutes of each other. Both were textually
|
|
clean merges that would have been semantically wrong in production.
|
|
Modes C and D were added later in the same rebase, from `services.py`.
|
|
|
|
## The four failure modes
|
|
|
|
Git resolves _text_. Neither of these produces a conflict marker.
|
|
|
|
### A. Missed application
|
|
|
|
Upstream establishes an invariant and applies it at every call site **it
|
|
knows about**. The fork has additional call sites upstream cannot see,
|
|
so the invariant silently does not hold there.
|
|
|
|
> **Worked example.** v1.6.8 made `event.amount_tickets` a per-wave
|
|
> roll-up recomputed by `sync_event_ticket_waves`, and added that call to
|
|
> `get_event` and `get_events` in `crud.py`. Both merged cleanly. But the
|
|
> fork has four getters upstream never had — `get_all_events`,
|
|
> `get_public_events`, `get_pending_events`,
|
|
> `get_events_pending_republish` — and two of them _publish_
|
|
> (`/republish-all` and the #55 sweep). Without the same call they emit
|
|
> the stale roll-up, so waves would have been wrong on exactly the paths
|
|
> that push to relays, and nowhere else.
|
|
|
|
### B. Changed meaning
|
|
|
|
Upstream redefines what an existing field _means_. Fork code that reads
|
|
it is untouched by the diff and keeps compiling, while now saying
|
|
something false.
|
|
|
|
> **Worked example.** After `sync_event_ticket_waves`,
|
|
> `event.price_per_ticket` is the **primary (first)** wave's price and
|
|
> `event.amount_tickets` is the **sum across all waves**. Our
|
|
> `nostr_publisher.build_nip52_event` reads both. Merged untouched, it
|
|
> would advertise the early-bird price after early-bird closed, and count
|
|
> stock in waves that have not opened. See #61.
|
|
|
|
### C. Misplaced conflict boundary
|
|
|
|
Git anchors a conflict on whatever lines happen to match. When both sides
|
|
rewrote the same region, an _incidental_ shared line inside it can become
|
|
the anchor — and everything past that line lands **outside** the markers,
|
|
where it reads as cleanly merged.
|
|
|
|
Resolving only what sits between the markers then leaves **both**
|
|
implementations in the file. Python does not complain: the later `def`
|
|
silently wins. Since the merge appends upstream after ours, the survivor
|
|
is upstream's — the fork's version is shadowed without a single warning.
|
|
|
|
> **Worked example.** In `services.py` the notification stack conflicted.
|
|
> Ours (220 lines: multipart HTML mail, the QR-card attachment, the
|
|
> Date/Message-ID headers that keep SpamAssassin quiet, npub DM support)
|
|
> appeared between the markers; upstream's showed as 4 lines. But both
|
|
> sides define the _same nine functions_, and git had anchored on a
|
|
> shared `_send_nostr_ticket_notification` line — so upstream's entire
|
|
> parallel stack sat below `>>>>>>>`, looking merged. Taking "ours" and
|
|
> moving on would have left upstream's definitions last in the file and
|
|
> therefore live, quietly reverting every one of those features.
|
|
|
|
**Do not trust the marker as the edit's boundary.** Before resolving a
|
|
hunk, list the function names on each side and compare them to the names
|
|
already in the file:
|
|
|
|
```sh
|
|
grep -o '^\(async \)\?def \w*' <file>.py | sed 's/.*def //' | sort | uniq -d
|
|
```
|
|
|
|
Run that per file **as you resolve**, not at the end. `ruff` does not
|
|
flag redefinition (verified: F ruleset passes on a duplicated `def`).
|
|
Only `mypy` does, via `no-redef` — and mypy refuses to run at all while
|
|
any file in the package still has conflict markers, so the one tool that
|
|
catches mode C is unavailable for the whole merge. The `uniq -d` line
|
|
above is the substitute.
|
|
|
|
### D. Collided names
|
|
|
|
Ours and upstream independently grew a function with the **same name for
|
|
a different feature**. Every resolution that reads as sane — take ours,
|
|
take theirs, take "the newer one" — silently deletes a feature, and the
|
|
diff looks like an ordinary reconciliation of one function.
|
|
|
|
> **Worked example.** Both sides had `_ticket_image_url`. Ours: the
|
|
> rendered QR ticket-card PNG, always attached, served by this extension
|
|
> off `lnbits_baseurl`. Upstream's: an organiser-uploaded template, opt-in
|
|
> per wave via `use_ticket_image`, served off `ticket_base_url` and
|
|
> returning `None` when the wave has not enabled it. Same name, different
|
|
> arity, different return type, unrelated features. Resolved by renaming
|
|
> ours to `_ticket_card_url` and keeping both — upstream's ticket-image
|
|
> upload UI had already merged into `index.vue`, so dropping their backend
|
|
> would have orphaned live UI.
|
|
|
|
Signals worth stopping on: the two versions differ in **arity**, in
|
|
**return type** (`str` vs `str | None`), or in which setting they build a
|
|
URL from. Any of those means it is probably not one function with two
|
|
histories.
|
|
|
|
## The procedure
|
|
|
|
Run this _after_ the merge resolves and _before_ the release.
|
|
|
|
### 1. Enumerate what upstream introduced
|
|
|
|
```sh
|
|
MB=$(git merge-base HEAD upstream/main)
|
|
# new public names
|
|
git diff $MB..<tag> -- '*.py' | grep -E "^\+(def |class |async def )"
|
|
# fields whose meaning was redefined
|
|
git show <tag>:models.py | sed -n '/^def sync_event_ticket_waves/,/return event/p'
|
|
```
|
|
|
|
Split the result into **new symbols** (mode A candidates) and
|
|
**redefined fields** (mode B candidates).
|
|
|
|
### 2. For each new symbol upstream _calls_, find the fork-only siblings
|
|
|
|
Ask what category of place the call belongs to — "every function that
|
|
returns an `Event` from the DB", "every path that prices a ticket" — then
|
|
enumerate that whole category in the merged tree and check coverage.
|
|
|
|
```sh
|
|
grep -n "sync_event_ticket_waves" crud.py # where upstream put it
|
|
grep -n "^async def get_.*-> \(list\[\)\?Event" crud.py # where it belongs
|
|
```
|
|
|
|
The gap between those two lists is the work.
|
|
|
|
### 3. For each redefined field, grep the fork-only files
|
|
|
|
The 30-odd files upstream has never seen are where mode B hides, because
|
|
nothing in the diff touches them:
|
|
|
|
```sh
|
|
git diff --name-only $MB..HEAD > /tmp/fork.txt
|
|
git diff --name-only $MB..<tag> > /tmp/up.txt
|
|
comm -23 <(sort /tmp/fork.txt) <(sort /tmp/up.txt) # fork-only files
|
|
grep -n "price_per_ticket\|amount_tickets" $(comm -23 ...)
|
|
```
|
|
|
|
### 4. Prove each finding before fixing it
|
|
|
|
Both examples above were confirmed by reading the code path end to end,
|
|
not inferred from the diff. A wrong theory costs more than the check:
|
|
during this rebase an inference that `amount_tickets` "goes stale on
|
|
sale" was wrong — `sync_event_ticket_waves` also runs on _reads_, which
|
|
only the call-site list showed.
|
|
|
|
### 5. Write the reason at the site
|
|
|
|
Every fix from this procedure gets a comment saying **why upstream's diff
|
|
missed it**. That is what stops the next rebase re-dropping it, and it is
|
|
the only durable record that the omission was considered rather than
|
|
overlooked.
|
|
|
|
## Checklist
|
|
|
|
- [ ] `migrations.py` still byte-identical to upstream
|
|
- [ ] New upstream symbols enumerated; each call-site category audited
|
|
- [ ] Redefined fields enumerated; every fork-only reader checked
|
|
- [ ] Fork-only files listed and grepped for both modes
|
|
- [ ] Every resolved file checked for duplicate `def`s (mode C) — as it is
|
|
resolved, since mypy cannot run until the whole package is clean
|
|
- [ ] Same-named functions on both sides compared by arity and return type
|
|
before being treated as one function (mode D)
|
|
- [ ] Publishing paths specifically audited — they fail silently and
|
|
externally, so they are the worst place for either mode to land
|
|
- [ ] Every fix carries a comment explaining the omission
|
|
- [ ] Deviations recorded in `docs/upstream-candidates.md`
|