Merge upstream v1.6.8 into the aio fork
Brings in ticket waves (per-wave price/currency/stock/fiat), the paginated ticket endpoint, the organiser ticket-image template, and the SatsPay on-chain surface. Refs #33. Resolutions that were not mechanical, and why: - set_ticket_paid debits the wave named on the ticket, keeping upstream's `> 0` guards; ours decremented unconditionally and could go negative. The purchase and free-ticket paths now stamp ticket_wave_id / ticket_wave_title, without which every sale would debit the primary wave. - Pricing moved onto the selected wave (basket_totals takes it as a required argument, the promo-validate endpoint resolves the same wave through the shared _resolve_ticket_wave). event.price_per_ticket is a roll-up of the PRIMARY wave since sync_event_ticket_waves, so pricing off the event quoted and charged the first wave's price to buyers who picked a later one. Regression test added. - Kept npub support in two places upstream removed it: the purchase endpoint's normalize_public_key path and the notification dispatcher. Upstream's replacement rejects with "Only NIP-05 Nostr identifiers are supported", which is false for this fork. The purchase-side rejection had merged in outside any conflict marker. - _ticket_image_url existed on both sides as two unrelated features. Ours (always-attached rendered QR card) is now _ticket_card_url; upstream's (organiser template, opt-in per wave) keeps the name. Both are wired into the mail, and the /qr/{ticket_id} endpoint — also duplicated on both sides, on the same route — is merged into one handler rather than registered twice, where the second copy would have been unreachable. - models._parse_date now accepts a full ISO datetime. Upstream's date-only strptime raised ValueError on any event whose closing_date carries a time, which create_event produces by defaulting it from event_end_date — it would have 500'd the purchase path, the public event gate and the promo preview. Reproduced before fixing. - Dropped upstream's inline make_qr_png (we import a superset from .qr) and its duplicate paymentMethodOptions in display.js, which re-derived payment options from per-method booleans and offered an on-chain option the backend rejects; the submit gate now matches the template's condition. - Restored imports the merge silently dropped with upstream's npub removal (normalize_public_key, normalize_private_key, DEFAULT_NOSTR_RELAYS). Event create/update stays ours: upstream's combined endpoint would have replaced the approval workflow and the explicit field allowlist that keeps `status` out of the request body. mypy error set is unchanged from HEAD; ruff, black and the 97 tests pass.
This commit is contained in:
commit
ae5affa44f
14 changed files with 2228 additions and 275 deletions
173
docs/rebase-playbook.md
Normal file
173
docs/rebase-playbook.md
Normal file
|
|
@ -0,0 +1,173 @@
|
|||
# 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`
|
||||
Loading…
Add table
Add a link
Reference in a new issue