Compare commits
26
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e16bd245bd | ||
|
|
887a9324b4 | ||
|
|
fcb484ead5 | ||
|
|
48de41cd69 | ||
|
|
66a6ee63ab | ||
|
|
10660c8168 | ||
|
|
441b67daaa | ||
|
|
026f26bdf6 | ||
|
|
73dc80bdc9 | ||
|
|
760021ea5a | ||
|
|
63ec068add | ||
|
|
a2ff0aed4c | ||
|
|
12e75ee24c | ||
|
|
792e87298b | ||
|
|
266e7032dd | ||
|
|
d6b48fb3ac | ||
|
|
3bf27e3fd5 | ||
|
|
185eb1b125 | ||
|
|
b3556d825c | ||
|
|
bf4f352117 | ||
|
|
1062b7c0bc | ||
|
|
48abecb830 | ||
|
|
e1c07438e9 | ||
|
|
6e563f3846 | ||
|
|
186f6a5839 | ||
|
|
786d9c6110 |
@@ -128,12 +128,26 @@ jobs:
|
|||||||
token="${ACTIONS_TOKEN:-$PACKAGE_TOKEN}"
|
token="${ACTIONS_TOKEN:-$PACKAGE_TOKEN}"
|
||||||
[ -n "$ACTIONS_TOKEN" ] || echo "note: GITEA_TOKEN is unset; using the PAT"
|
[ -n "$ACTIONS_TOKEN" ] || echo "note: GITEA_TOKEN is unset; using the PAT"
|
||||||
|
|
||||||
|
# **On the parent, not on HEAD.** The floor marks what has
|
||||||
|
# already been released, so tagging the commit being pushed
|
||||||
|
# leaves nothing between the floor and HEAD — semantic-release
|
||||||
|
# then correctly reports there is nothing to release, which is
|
||||||
|
# exactly what the first run of this workflow did. HEAD^ is the
|
||||||
|
# first parent, so on the merge commit this fires for it is main
|
||||||
|
# as it was before the merge, and everything the merge brought
|
||||||
|
# in is releasable.
|
||||||
|
floor=$(git rev-parse "${{ github.sha }}^" 2>/dev/null || true)
|
||||||
|
if [ -z "$floor" ]; then
|
||||||
|
echo "HEAD has no parent, so no commit can precede the floor" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
echo "no v* tag exists — seeding v0.0.0 so the first release is 0.0.1"
|
echo "no v* tag exists — seeding v0.0.0 so the first release is 0.0.1"
|
||||||
git tag v0.0.0 "${{ github.sha }}"
|
git tag v0.0.0 "$floor"
|
||||||
git push --quiet \
|
git push --quiet \
|
||||||
"https://x-access-token:${token}@${SERVER_URL#https://}/${REPO}.git" \
|
"https://x-access-token:${token}@${SERVER_URL#https://}/${REPO}.git" \
|
||||||
refs/tags/v0.0.0
|
refs/tags/v0.0.0
|
||||||
echo "seeded v0.0.0 at ${{ github.sha }}"
|
echo "seeded v0.0.0 at $floor (parent of ${{ github.sha }})"
|
||||||
|
|
||||||
# Pinned rather than installed into the repo: this is a Go project
|
# Pinned rather than installed into the repo: this is a Go project
|
||||||
# and a package.json at its root invites the npm plugin and every
|
# and a package.json at its root invites the npm plugin and every
|
||||||
|
|||||||
@@ -3445,3 +3445,41 @@ their own output, and leave the previous snapshots in place.
|
|||||||
makes it worth having: a restored snapshot resolves to `refresh` and
|
makes it worth having: a restored snapshot resolves to `refresh` and
|
||||||
folds in the incremental listens since — minutes, against the 3–23 h a
|
folds in the incremental listens since — minutes, against the 3–23 h a
|
||||||
rebuild was estimating.
|
rebuild was estimating.
|
||||||
|
|
||||||
|
## A green release pipeline can ship an empty changelog (2026-08-18)
|
||||||
|
|
||||||
|
`conventional-changelog-conventionalcommits@10` is silently incompatible
|
||||||
|
with the writer `@semantic-release/release-notes-generator@14` depends on
|
||||||
|
(`conventional-changelog-writer@^8`). Every release note renders as a bare
|
||||||
|
`## 0.0.1 (date)` heading with **no sections and no commits under it**, no
|
||||||
|
step fails, and the release ships with an empty body.
|
||||||
|
|
||||||
|
It is pinned to `9` in `.gitea/workflows/release.yml` and in
|
||||||
|
`make release-dry`, which must stay identical. **Check the rendered notes,
|
||||||
|
never the exit code** — this is invisible to every tick in the pipeline.
|
||||||
|
|
||||||
|
## semantic-release needs push rights to the branch even when it never pushes to it (2026-08-18)
|
||||||
|
|
||||||
|
Core runs `git push --dry-run HEAD:<branch>` as a permission check, before
|
||||||
|
and independently of any plugin. With `@semantic-release/git` removed
|
||||||
|
nothing ever pushes to `main`, and the check still runs.
|
||||||
|
|
||||||
|
Two things this looked like and was not:
|
||||||
|
|
||||||
|
- **Not branch protection.** A `--dry-run` push does not reach the
|
||||||
|
pre-receive hook: pushing one to protected `main` with a write-scoped
|
||||||
|
token succeeds. So `main`'s `enable_push: false` is not what fails here.
|
||||||
|
- **A flat `403 Forbidden`, not Gitea's protection message.** That is the
|
||||||
|
tell. `PACKAGE_TOKEN` had package-write and repo-*read* — enough to
|
||||||
|
clone a private repo, so every other workflow was fine — and needed
|
||||||
|
`write:repository`.
|
||||||
|
|
||||||
|
## A tag-triggered workflow runs the workflow file at the *tagged* commit (2026-08-18)
|
||||||
|
|
||||||
|
Not the one on `main`. Moving `v0.0.0` onto a pre-merge commit ran that
|
||||||
|
commit's version of `homebrew-formula.yml`, which predated the `v0.0.0`
|
||||||
|
skip guard added in the same plan, and it pushed a `0.0.0` formula to the
|
||||||
|
public tap.
|
||||||
|
|
||||||
|
A guard added today does not protect a tag that points at yesterday. When
|
||||||
|
re-pointing a tag, check what the workflows looked like *there*.
|
||||||
|
|||||||
@@ -1,358 +0,0 @@
|
|||||||
# 017 — Releases that happen by themselves
|
|
||||||
|
|
||||||
> **Status: built, not yet run.** Phases 0–4 have landed on this branch;
|
|
||||||
> phase 5 is the merge itself and cannot be done until then. The old
|
|
||||||
> `v1.x` tags are already deleted from `origin`. Verified locally against
|
|
||||||
> a scratch remote: semantic-release computes **0.0.1** from these
|
|
||||||
> commits and renders correct sectioned notes.
|
|
||||||
>
|
|
||||||
> **One thing found by testing that no amount of reading would have
|
|
||||||
> caught.** `conventional-changelog-conventionalcommits@10` — the current
|
|
||||||
> release, and my first pin — is silently incompatible with the writer
|
|
||||||
> `release-notes-generator@14` depends on: the version is right, the tag
|
|
||||||
> is right, every step reports success, and the release body is a bare
|
|
||||||
> `## 0.0.1 (date)` heading with **nothing under it**. It is pinned to 9
|
|
||||||
> in both `release.yml` and `make release-dry`, with the reason written
|
|
||||||
> beside it. Four of my seven original pins were wrong majors besides;
|
|
||||||
> they were guesses, and `npm view` was the fix.
|
|
||||||
|
|
||||||
The goal in one sentence: **a merge to `main` computes the next version
|
|
||||||
from the commits it contains, cuts a tag and a Gitea release whose body
|
|
||||||
is the changelog, and every publishing channel builds that tag.** The
|
|
||||||
first release under this scheme is `v0.0.1`, and the five existing `v1.x`
|
|
||||||
tags go.
|
|
||||||
|
|
||||||
## What is there now
|
|
||||||
|
|
||||||
Measured, not remembered:
|
|
||||||
|
|
||||||
- **Five tags and zero releases.** `v1.3.0`, `v1.4.0`, `v1.4.1`,
|
|
||||||
`v1.5.0`, `v1.6.0` exist on `origin`;
|
|
||||||
`GET /api/v1/repos/yonlu/yellowjacket/releases` returns `[]`. So there
|
|
||||||
is no release page to preserve and nothing but the tags to remove.
|
|
||||||
- **`CHANGELOG.md` is stale and belongs to another repo.** Its newest
|
|
||||||
entry is `1.3.0` and every link in it points at
|
|
||||||
`github.com/onion-4-dinner/yellowjacket` — it was written by a
|
|
||||||
semantic-release run against a GitHub remote this project no longer
|
|
||||||
has.
|
|
||||||
- **`.releaserc.yml` is a complete semantic-release config that nothing
|
|
||||||
invokes**, which CLAUDE.md already says in as many words.
|
|
||||||
- **Root `package.json` is literally `{}`** — the stub left behind by
|
|
||||||
whatever was going to run it.
|
|
||||||
- The triggers today are: `arch-package` on **push to `main`**,
|
|
||||||
`homebrew-formula` on **`v*`**, `android-apk` on **`v*`**, `ci` on
|
|
||||||
every branch, `index-artifact` on cron/dispatch. So Arch publishes a
|
|
||||||
`git describe` version on every merge and the other two publish only
|
|
||||||
when a human remembers to push a tag.
|
|
||||||
|
|
||||||
## Decision 1 — semantic-release, with `exec` in place of the `github` plugin
|
|
||||||
|
|
||||||
**Revised: the first draft of this plan proposed a shell script and the
|
|
||||||
argument for it does not hold.** Recorded here rather than deleted,
|
|
||||||
because the reasoning is what the decision rests on.
|
|
||||||
|
|
||||||
What I said, and what checking it showed:
|
|
||||||
|
|
||||||
- *"The two plugins that would carry the work do not fit."* Half true.
|
|
||||||
`@semantic-release/github` genuinely does not speak Gitea's `/api/v1`
|
|
||||||
— but the replacement is **`@semantic-release/exec`**, which is
|
|
||||||
first-party, published 2026-06, and peer-deps `semantic-release >=24.1`.
|
|
||||||
Its `publishCmd` is one `curl` at the Gitea release endpoint with
|
|
||||||
`${nextRelease.notes}` as the body. The Gitea-shaped part of this is
|
|
||||||
five lines, and the part I proposed to hand-roll — parsing conventional
|
|
||||||
commits, ordering semver, rendering grouped notes — is the part with
|
|
||||||
the edge cases and none of it is Gitea-shaped at all.
|
|
||||||
- *"`@semantic-release/git` commits the changelog back to `main`, which
|
|
||||||
re-triggers everything."* True, and it is the one real risk — but it
|
|
||||||
is a two-line guard (skip the job when `HEAD`'s subject is
|
|
||||||
`chore(release):`), not a reason to write a version calculator. That
|
|
||||||
guard is needed under **either** design, since either one writes a
|
|
||||||
changelog commit.
|
|
||||||
- *"A Node dependency tree at the root of a Go repo."* The commitlint
|
|
||||||
precedent does not transfer. commitlint was a dependency to regex one
|
|
||||||
line; this is a dependency to do something with real complexity, it is
|
|
||||||
`npx`-only so nothing lands in the repo, and Node is already installed
|
|
||||||
in CI for the frontend.
|
|
||||||
- *"It cannot be told to produce `0.0.1`."* Wrong — that is a property
|
|
||||||
of which commits are in the range, not of the tool. Identical under
|
|
||||||
both designs. See below.
|
|
||||||
|
|
||||||
Note also that **`@saithodev/semantic-release-gitea` is a dead end** and
|
|
||||||
should not be reached for: last published 2022, depends on `got@10` and
|
|
||||||
`fs-extra@8`, and declares no peer dependency on semantic-release at all
|
|
||||||
— i.e. it is untested against anything since v19, against a core now at
|
|
||||||
v25. `exec` + `curl` is both simpler and maintained.
|
|
||||||
|
|
||||||
So `.releaserc.yml` stays, and its plugin list becomes five **first-party**
|
|
||||||
plugins, all published within the last six months:
|
|
||||||
|
|
||||||
| plugin | job |
|
|
||||||
| --- | --- |
|
|
||||||
| `commit-analyzer` | the version |
|
|
||||||
| `release-notes-generator` | the notes |
|
|
||||||
| `changelog` | writes `CHANGELOG.md` |
|
|
||||||
| `git` | commits it back |
|
|
||||||
| `exec` | `curl`s the Gitea release |
|
|
||||||
|
|
||||||
The `releaseRules` and `presetConfig` blocks already in the file are
|
|
||||||
kept verbatim — they are the same bump table `commit-check.sh` already
|
|
||||||
enforces the grammar for, and nothing about the project's commit
|
|
||||||
convention changes.
|
|
||||||
|
|
||||||
Two mechanical details that decide whether this works at all:
|
|
||||||
|
|
||||||
- **semantic-release pushes the tag itself**, as core behaviour, using
|
|
||||||
`repositoryUrl`. The remote here is `ssh://git@git.ljones.me:2222/…`,
|
|
||||||
which would need an SSH key in CI — so the run passes
|
|
||||||
`--repository-url "https://x-access-token:$PACKAGE_TOKEN@git.ljones.me/yonlu/yellowjacket.git"`
|
|
||||||
on the command line rather than committing a token to the config.
|
|
||||||
**That is also what satisfies Decision 2**: the tag push is attributed
|
|
||||||
to a real user, not to the Actions token.
|
|
||||||
- **The empty root `package.json` (`{}`) goes.** semantic-release does
|
|
||||||
not need one when `--repository-url` is explicit, and leaving a
|
|
||||||
package manifest at the root of a Go repo invites the npm plugin and
|
|
||||||
every tool that looks for one.
|
|
||||||
|
|
||||||
Invocation is pinned in the workflow, not installed into the repo:
|
|
||||||
|
|
||||||
```
|
|
||||||
npx --yes \
|
|
||||||
-p semantic-release@25 \
|
|
||||||
-p @semantic-release/commit-analyzer@14 \
|
|
||||||
-p @semantic-release/release-notes-generator@15 \
|
|
||||||
-p @semantic-release/changelog@6 \
|
|
||||||
-p @semantic-release/git@10 \
|
|
||||||
-p @semantic-release/exec@7 \
|
|
||||||
-p conventional-changelog-conventionalcommits@9 \
|
|
||||||
semantic-release --repository-url "…"
|
|
||||||
```
|
|
||||||
|
|
||||||
(Exact majors get pinned from `npm view` at implementation time;
|
|
||||||
`conventional-changelog-conventionalcommits` is in the list because both
|
|
||||||
the analyzer and the notes generator name that preset and neither
|
|
||||||
depends on it.)
|
|
||||||
|
|
||||||
## Decision 2 — how the publish workflows learn about the tag
|
|
||||||
|
|
||||||
**Gitea, like GitHub, does not start a workflow from a tag pushed by a
|
|
||||||
workflow's own token** (go-gitea#33123, and the forum thread it points
|
|
||||||
at). This is the one load-bearing unknown in the plan.
|
|
||||||
|
|
||||||
The remedy is to push the tag with a *user* PAT — `secrets.PACKAGE_TOKEN`
|
|
||||||
is already in this repo and already used by `arch-package` and
|
|
||||||
`android-apk` to clone and to publish — so the push is attributed to a
|
|
||||||
person and the `v*` triggers fire normally. That keeps the three publish
|
|
||||||
workflows completely unchanged in shape.
|
|
||||||
|
|
||||||
**It is verified in phase 5, not assumed.** The fallback, if it does not
|
|
||||||
fire, is an explicit `POST
|
|
||||||
/api/v1/repos/{owner}/{repo}/actions/workflows/{file}/dispatches` per
|
|
||||||
channel from the release job. That needs `workflow_dispatch` (with a
|
|
||||||
`version` input) added to `homebrew-formula.yml` and `arch-package.yml`;
|
|
||||||
`android-apk.yml` already has both. **Add those inputs in phase 3
|
|
||||||
regardless** — a hand-triggered rebuild of one channel is worth having
|
|
||||||
whether or not the fallback is needed.
|
|
||||||
|
|
||||||
The alternative — one `release.yml` with the three publishes as
|
|
||||||
`needs:` jobs — is rejected: it means either copying ~400 lines of
|
|
||||||
Android and Arch setup into it or relying on `workflow_call`, and it
|
|
||||||
puts every merge to `main` behind an up-to-60-minute Android build on a
|
|
||||||
runner with capacity 1.
|
|
||||||
|
|
||||||
## Decision 3 — 1.6.0 → 0.0.1 is a downgrade, and the answer is reinstall
|
|
||||||
|
|
||||||
**Decided: no version-code offset, no epoch. The version number stays
|
|
||||||
honest and existing installs are replaced by hand.** Every channel is a
|
|
||||||
downgrade and each declines differently, so what to expect:
|
|
||||||
|
|
||||||
- **Arch: no upgrade is offered, silently.** `pkgver()` derives from
|
|
||||||
`git describe`, so after the wipe it reads `0.0.1.rN.gHASH`, which
|
|
||||||
pacman orders *below* the `1.3.0.rN.*` in the registry. `pacman -R
|
|
||||||
yellowjacket && pacman -S yellowjacket` is the remedy. (`epoch=1` in
|
|
||||||
the PKGBUILD would have avoided it for one line — but an epoch can
|
|
||||||
never be removed, and it puts a permanent `1:` in front of every
|
|
||||||
version string this project will ever have.)
|
|
||||||
- **Homebrew: no upgrade is offered, silently.** Brew has no epoch at
|
|
||||||
all. `brew uninstall yellowjacket && brew install …`.
|
|
||||||
- **Android: a hard refusal.** `versionCode` is
|
|
||||||
`maj*10000 + min*100 + pat`, so `0.0.1` is **1** against the **10300**
|
|
||||||
an installed 1.3.0 carries, and the install fails with
|
|
||||||
`INSTALL_FAILED_VERSION_DOWNGRADE`. Uninstall first — **and that takes
|
|
||||||
the app's library and config with it**, which is the same data loss
|
|
||||||
`android-apk.yml`'s keystore guard exists to prevent, arrived at from
|
|
||||||
the other direction. The workflow's own `code -le 0` guard still passes
|
|
||||||
at 1, so nothing in CI stops or warns about this.
|
|
||||||
|
|
||||||
All three go in the release notes for `v0.0.1` and in
|
|
||||||
`packaging/homebrew/README.md` / `docs/android-release.md`, because a
|
|
||||||
channel that silently offers no upgrade is indistinguishable from a
|
|
||||||
broken pipeline six months from now.
|
|
||||||
|
|
||||||
## Landing exactly `v0.0.1`
|
|
||||||
|
|
||||||
Determinism comes from two things:
|
|
||||||
|
|
||||||
1. **Seed `v0.0.0` on `6fb7b5e`** (current `origin/main`) after wiping
|
|
||||||
the old tags. That is the floor, and the analyser's range starts
|
|
||||||
there.
|
|
||||||
2. **This branch carries no `feat:` commit.** Everything in it is
|
|
||||||
`ci:`/`docs:`/`chore:`/`build:`, plus at least one `fix:` — which is
|
|
||||||
honest, since wiring up release machinery that was configured and
|
|
||||||
never run *is* a fix. One patch-level commit in `v0.0.0..HEAD`
|
|
||||||
computes `0.0.1` and nothing else can.
|
|
||||||
|
|
||||||
This is a property of the commit range, not of the tool — it would have
|
|
||||||
been the same constraint under the shell script.
|
|
||||||
|
|
||||||
This is a real constraint on the branch, not an accounting trick: a
|
|
||||||
single `feat:` commit here makes the first release `v0.1.0`.
|
|
||||||
|
|
||||||
`v0.0.0` itself gets no release object — it is a floor, not a shipment.
|
|
||||||
|
|
||||||
## Decision 4 — what the release page carries
|
|
||||||
|
|
||||||
Four artifacts, and the fourth is the interesting one. Measured on this
|
|
||||||
machine rather than assumed:
|
|
||||||
|
|
||||||
| asset | built by | state |
|
|
||||||
| --- | --- | --- |
|
|
||||||
| `yellowjacket-<v>-android-arm64.apk` | `android-apk.yml` | already built, verified, signed |
|
|
||||||
| `yellowjacket-<v>-linux-amd64.tar.gz` | new job | binary + `.desktop` + icon |
|
|
||||||
| `yellowjacket-<v>-x86_64.pkg.tar.zst` | `arch-package.yml` | already built; free to attach |
|
|
||||||
| `yellowjacket-<v>-windows-amd64.zip` | new job | **compiles; has never been run** |
|
|
||||||
|
|
||||||
**macOS cannot be one of them.** `GOOS=darwin CGO_ENABLED=0` fails at
|
|
||||||
`wails/v3/pkg/mac: build constraints exclude all Go files` — the darwin
|
|
||||||
backend is Objective-C behind cgo, so a `.app` needs a macOS host and
|
|
||||||
the runner is a Linux container. That is precisely why the Homebrew
|
|
||||||
channel builds from source on the user's own Mac, and it stays the
|
|
||||||
answer for macOS.
|
|
||||||
|
|
||||||
**Windows is newly possible and should be labelled honestly.**
|
|
||||||
`GOOS=windows GOARCH=amd64 CGO_ENABLED=0 go build -tags production`
|
|
||||||
succeeds in 2.5 s and produces a 40 MB `.exe` — nothing in the audio,
|
|
||||||
database or webview path needs cgo on Windows (oto uses WinMM through
|
|
||||||
`x/sys`, sqlite is modernc's pure-Go driver, WebView2 is COM syscalls,
|
|
||||||
and MPRIS is `linux && !android`-tagged). But **compiling is not
|
|
||||||
running**: no Windows build of this app has ever been started, no CI tier
|
|
||||||
can exercise one, and `backend/system`'s `%LOCALAPPDATA%` path has never
|
|
||||||
resolved on a real machine. It ships marked as untested in the release
|
|
||||||
notes, or it does not ship — an unlabelled Windows download is a promise
|
|
||||||
nothing here can keep.
|
|
||||||
|
|
||||||
### The race the ordering creates
|
|
||||||
|
|
||||||
semantic-release runs **prepare** (changelog commit, tag push) before
|
|
||||||
**publish** (the `exec` curl that creates the release object). The tag
|
|
||||||
push is what starts the publishing workflows — so a fast one can reach
|
|
||||||
its upload step *before the release exists*, and
|
|
||||||
`POST /releases/{id}/assets` needs an id.
|
|
||||||
|
|
||||||
The capacity-1 runner serialises things enough that this would usually
|
|
||||||
work, which is the worst kind of bug. So each upload step **polls
|
|
||||||
`GET /api/v1/repos/…/releases/tags/{tag}` with a bounded retry** before
|
|
||||||
uploading, and fails loudly on timeout rather than skipping the asset.
|
|
||||||
That is ~8 lines of shell, shared by all three publishers.
|
|
||||||
|
|
||||||
## Phases
|
|
||||||
|
|
||||||
**Phase 0 — clear the ground.**
|
|
||||||
Delete `v1.3.0`–`v1.6.0` locally and on `origin`; push `v0.0.0` at
|
|
||||||
`6fb7b5e` — this is the floor semantic-release reads, and without it the
|
|
||||||
first release is `1.0.0` by its own rule. Delete the empty root
|
|
||||||
`package.json`. Truncate `CHANGELOG.md` to a header plus a line saying
|
|
||||||
history before `0.0.1` is in `git log` — the existing content is another
|
|
||||||
repo's links and cannot be repaired, only replaced, and the `changelog`
|
|
||||||
plugin prepends to whatever it finds.
|
|
||||||
|
|
||||||
**Phase 1 — `.releaserc.yml`.**
|
|
||||||
Swap `@semantic-release/github` for `@semantic-release/exec`, whose
|
|
||||||
`publishCmd` POSTs to
|
|
||||||
`/api/v1/repos/yonlu/yellowjacket/releases` with `tag_name`, `name` and
|
|
||||||
`body` taken from `${nextRelease.*}`. Keep `commit-analyzer`,
|
|
||||||
`release-notes-generator`, `changelog` and `git` exactly as written; fix
|
|
||||||
the `git` plugin's commit message so it passes `commit-check`
|
|
||||||
(`chore(release): ${nextRelease.version}` — the existing one already
|
|
||||||
does, but the trailing `${nextRelease.notes}` in the body is worth
|
|
||||||
keeping deliberate rather than incidental). `make release-dry` wraps
|
|
||||||
`semantic-release --dry-run` so the next version is answerable without
|
|
||||||
pushing anything.
|
|
||||||
|
|
||||||
`scripts/commit-check.sh`'s header already points at `.releaserc.yml`
|
|
||||||
for the type list and stays correct — that coupling survives this plan
|
|
||||||
rather than being broken by it.
|
|
||||||
|
|
||||||
**Phase 2 — `.gitea/workflows/release.yml`.**
|
|
||||||
On `push: branches: [main]`. Node 22, the pinned `npx` line from
|
|
||||||
Decision 1, `--repository-url` carrying `PACKAGE_TOKEN`. Concurrency
|
|
||||||
group `release-main` with `cancel-in-progress: false` — cutting a tag is
|
|
||||||
not a thing to cancel halfway.
|
|
||||||
|
|
||||||
The one guard that matters: **the job exits early when `HEAD`'s subject
|
|
||||||
starts `chore(release):`**, so the changelog commit the `git` plugin
|
|
||||||
pushes cannot re-enter this workflow. That is checked in shell rather
|
|
||||||
than left to `[skip ci]`, whose handling in Gitea is one more thing that
|
|
||||||
would have to be verified.
|
|
||||||
|
|
||||||
**Phase 3 — rewire the publish workflows.**
|
|
||||||
`arch-package.yml` moves from `push: branches: [main]` to
|
|
||||||
`push: tags: ['v*']` plus `workflow_dispatch`, so a merge no longer
|
|
||||||
publishes an untagged package. `homebrew-formula.yml` gains
|
|
||||||
`workflow_dispatch` with a `version` input and takes its version from
|
|
||||||
the input when there is no tag. `android-apk.yml` needs neither.
|
|
||||||
|
|
||||||
**Phase 3b — the assets.**
|
|
||||||
`scripts/release-asset.sh` is the shared uploader: wait for the release
|
|
||||||
by tag, then `POST /releases/{id}/assets?name=…`. `android-apk.yml` and
|
|
||||||
`arch-package.yml` each call it with the artifact they already built.
|
|
||||||
A new `desktop-assets` job — `push: tags: ['v*']`, in the same
|
|
||||||
`ubuntu:24.04` container `ci.yml` uses — builds the Linux binary via
|
|
||||||
`make build-prod` and the Windows one via the `CGO_ENABLED=0`
|
|
||||||
cross-compile, and uploads both. It is a separate job from the Arch one
|
|
||||||
because that runs in an `archlinux` container as an unprivileged
|
|
||||||
`makepkg` user, and grafting two unrelated builds onto it would make one
|
|
||||||
failure look like the other.
|
|
||||||
|
|
||||||
**Phase 4 — say that the upgrade is a reinstall, and that Windows is untried.**
|
|
||||||
No code change: a note in `packaging/homebrew/README.md`, one in
|
|
||||||
`docs/android-release.md`, the three-channel downgrade warning written
|
|
||||||
into the `v0.0.1` release notes, and a standing line in the notes
|
|
||||||
template marking the Windows asset unverified until someone runs it.
|
|
||||||
|
|
||||||
**Phase 5 — cut it and watch.** *(the only phase left)*
|
|
||||||
Merge, then verify with `gitea_ci` that (a) `release.yml` ran, seeded
|
|
||||||
`v0.0.0` and produced `v0.0.1`, (b) the release exists **with a non-empty
|
|
||||||
body** — check the body, not the exit code — and (c) **all four publish
|
|
||||||
workflows started from the tag**. If (c) is empty, that is Decision 2's
|
|
||||||
fallback and the `workflow_dispatch` inputs added in phase 3 are already
|
|
||||||
there to drive it.
|
|
||||||
|
|
||||||
The expected sequence on the merge is: `release.yml` seeds `v0.0.0`
|
|
||||||
(triggering nothing), releases `0.0.1`, and pushes both the changelog
|
|
||||||
commit and the tag — at which point `release.yml` fires a second time on
|
|
||||||
the changelog commit and exits at the `chore(release):` guard, while the
|
|
||||||
four `v*` workflows start. On a capacity-1 runner they will queue behind
|
|
||||||
each other, Android last and longest.
|
|
||||||
|
|
||||||
**Phase 6 — the documentation that will otherwise be wrong.**
|
|
||||||
CLAUDE.md's *Commits* section currently explains `.releaserc.yml` and
|
|
||||||
says nothing runs it; the CI section says there are five workflows and
|
|
||||||
that only `ci.yml` gates. Both change. `docs/android-release.md`
|
|
||||||
describes tags as hand-pushed. `make skill-check` fails on a `.pi/`
|
|
||||||
reference to a make target that does not exist, so `make release-dry`
|
|
||||||
gets documented or nothing does.
|
|
||||||
|
|
||||||
## Open questions for you
|
|
||||||
|
|
||||||
1. **Ship the Windows `.exe` or not?** It builds, and it has never run.
|
|
||||||
Marked-as-untested is the assumption; say if you would rather hold it
|
|
||||||
back until someone boots it.
|
|
||||||
|
|
||||||
Resolved: semantic-release stays, with `exec` in place of the `github`
|
|
||||||
plugin (Decision 1). Reinstalls are accepted, so no epoch and no
|
|
||||||
versionCode offset (Decision 3). The release carries the APK, a Linux
|
|
||||||
tarball, the Arch package and — pending (1) — a Windows zip; macOS is
|
|
||||||
not buildable here and stays a Homebrew-from-source channel (Decision 4).
|
|
||||||
`v0.0.0` has to be a real tag under this design — semantic-release reads
|
|
||||||
git tags for its floor and has no "treat absence as 0.0.0" knob that
|
|
||||||
also stops it calling the first release `1.0.0`.
|
|
||||||
@@ -0,0 +1,87 @@
|
|||||||
|
# 017 — Releases that happen by themselves
|
||||||
|
|
||||||
|
**Shipped as `v0.0.1`.** A merge to `main` now reads the Conventional
|
||||||
|
Commits since the last tag, cuts the tag and the Gitea release whose body
|
||||||
|
is the generated changelog, and the four publishing workflows build that
|
||||||
|
tag and attach their artifacts. Nothing is released by hand.
|
||||||
|
|
||||||
|
## What it looks like now
|
||||||
|
|
||||||
|
`release.yml` on push to `main` → semantic-release → tag → four `v*`
|
||||||
|
workflows in parallel (serialised in practice by the capacity-1 runner):
|
||||||
|
|
||||||
|
| workflow | publishes | attaches |
|
||||||
|
| --- | --- | --- |
|
||||||
|
| `arch-package` | pacman registry | `…-x86_64.pkg.tar.zst` |
|
||||||
|
| `android-apk` | generic registry (Obtainium) | `…-android-arm64.apk` |
|
||||||
|
| `desktop-assets` | — | `…-linux-amd64.tar.gz` |
|
||||||
|
| `homebrew-formula` | the public tap | — (builds from source) |
|
||||||
|
|
||||||
|
Verified on the real thing: all five green, three assets on the release,
|
||||||
|
the tap at `0.0.1`, and the Obtainium `latest` URL serving 200.
|
||||||
|
|
||||||
|
## The five decisions, and what they cost
|
||||||
|
|
||||||
|
1. **semantic-release, not a shell script.** The first draft of this plan
|
||||||
|
proposed hand-rolling it and the argument did not survive checking:
|
||||||
|
`@semantic-release/exec` is first-party and current, and the
|
||||||
|
Gitea-shaped part is one `curl`. What I would have hand-rolled —
|
||||||
|
commit parsing, semver ordering, note rendering — is the part with the
|
||||||
|
edge cases and none of it is Gitea-shaped.
|
||||||
|
2. **`@saithodev/semantic-release-gitea` is a dead end** and was offered
|
||||||
|
before it was checked: last published 2022, `got@10`, and no peer
|
||||||
|
dependency on semantic-release at all.
|
||||||
|
3. **No `@semantic-release/git`.** `main` is protected, so a changelog
|
||||||
|
commit-back is rejected by the pre-receive hook — and would be
|
||||||
|
rejected *after* the tag was pushed, leaving a tagged release the run
|
||||||
|
reports as failed. The release page is the changelog;
|
||||||
|
`.release-notes.md` is a gitignored carrier and `CHANGELOG.md` is a
|
||||||
|
signpost.
|
||||||
|
4. **Versions restart at `0.0.1`**, a downgrade on every channel. No
|
||||||
|
`epoch`, no `versionCode` offset: both are permanent, a reinstall is
|
||||||
|
once. Documented in `packaging/homebrew/README.md` and
|
||||||
|
`docs/android-release.md`.
|
||||||
|
5. **No macOS and no Windows.** `GOOS=darwin CGO_ENABLED=0` fails at
|
||||||
|
`wails/v3/pkg/mac` and there is no macOS runner, so Homebrew-from-source
|
||||||
|
stays that channel. Windows cross-compiles in ~2.5 s and is withheld
|
||||||
|
because no build of it has ever been *run*.
|
||||||
|
|
||||||
|
## Four things that only showed up by running it
|
||||||
|
|
||||||
|
- **`conventional-changelog-conventionalcommits@10` renders empty
|
||||||
|
notes.** Silently: right version, right tag, every step green, and a
|
||||||
|
release body that is a bare `## 0.0.1 (date)` heading with nothing
|
||||||
|
beneath it. Held at `9`, in `release.yml` and `make release-dry`, with
|
||||||
|
the reason beside both. **Check the rendered notes, never the exit
|
||||||
|
code.**
|
||||||
|
- **semantic-release core dry-run-pushes to the release branch** as a
|
||||||
|
permission check, independently of any plugin. `PACKAGE_TOKEN` had
|
||||||
|
package-write and repo-*read* — enough to clone, not enough for this —
|
||||||
|
and it failed with a flat `403 Forbidden` that reads exactly like
|
||||||
|
branch protection. It is not: a `--dry-run` push never reaches the
|
||||||
|
pre-receive hook, which a one-line experiment settled. The token needed
|
||||||
|
`write:repository`.
|
||||||
|
- **The floor tag must go on `HEAD^`, not `HEAD`.** Seeded on the merge
|
||||||
|
commit itself it leaves nothing between the floor and HEAD, and
|
||||||
|
semantic-release correctly reports there is nothing to release. The
|
||||||
|
first run did exactly that and cut nothing.
|
||||||
|
- **A tag-triggered workflow runs from the tagged commit's tree.**
|
||||||
|
Moving `v0.0.0` back to `6fb7b5e` ran the *pre-merge* homebrew
|
||||||
|
workflow, which predates the `v0.0.0` skip guard, and pushed a `0.0.0`
|
||||||
|
formula to the public tap. Self-corrected at `0.0.1`. The corollary is
|
||||||
|
general: a guard added today does not protect a tag pointing at
|
||||||
|
yesterday.
|
||||||
|
|
||||||
|
## Two mechanisms confirmed, having been assumptions
|
||||||
|
|
||||||
|
- **A tag pushed with a user PAT does start the `v*` workflows**; one
|
||||||
|
pushed with the Actions token does not (go-gitea#33123). Both halves
|
||||||
|
are load-bearing and both were observed: the floor seed triggered
|
||||||
|
nothing, and the release tag triggered all four.
|
||||||
|
- **Tags are not protected** on this repo, only `main` — which is what
|
||||||
|
lets semantic-release tag at all.
|
||||||
|
|
||||||
|
## Left behind deliberately
|
||||||
|
|
||||||
|
`v0.0.0` stays on `origin` as the floor. It carries no release, and all
|
||||||
|
four publishers skip it by name.
|
||||||
@@ -310,15 +310,35 @@ func (r *Reconciler) run(ctx context.Context, force bool) (Summary, error) {
|
|||||||
|
|
||||||
summary.Synced = r.syncExternalLists(ctx)
|
summary.Synced = r.syncExternalLists(ctx)
|
||||||
|
|
||||||
attempted, started, err := r.attemptDue(ctx, force)
|
// Nothing is searched for when there is nothing to search with, and
|
||||||
if err != nil {
|
// the point is what that *does not* do to the list.
|
||||||
return summary, err
|
//
|
||||||
|
// Attempting anyway is not merely wasted work: every request comes
|
||||||
|
// back "no download clients are enabled", which RecordAttempt writes
|
||||||
|
// down as an attempt and schedules a retry for -- so a user who has
|
||||||
|
// deliberately built a wanted list with no client watched their
|
||||||
|
// requests accrue failures and announce "next check in 6 hours"
|
||||||
|
// about a check that cannot happen. Wanting something without a way
|
||||||
|
// to fetch it is a supported thing to do; being told it is being
|
||||||
|
// looked for is a lie.
|
||||||
|
//
|
||||||
|
// Everything above this line still runs: an artist subscription
|
||||||
|
// still expands, and a request the user satisfied by some other
|
||||||
|
// route -- ripped, bought, copied in -- is still retired, because
|
||||||
|
// neither needs a provider.
|
||||||
|
summary.NoProviders = len(r.manager.enabledProviders()) == 0
|
||||||
|
|
||||||
|
if !summary.NoProviders {
|
||||||
|
attempted, started, err := r.attemptDue(ctx, force)
|
||||||
|
if err != nil {
|
||||||
|
return summary, err
|
||||||
|
}
|
||||||
|
|
||||||
|
summary.Attempted = attempted
|
||||||
|
summary.Started = started
|
||||||
}
|
}
|
||||||
|
|
||||||
summary.Attempted = attempted
|
|
||||||
summary.Started = started
|
|
||||||
summary.Waiting = r.countWaiting(ctx)
|
summary.Waiting = r.countWaiting(ctx)
|
||||||
summary.NoProviders = len(r.manager.enabledProviders()) == 0
|
|
||||||
|
|
||||||
r.logger.Info(
|
r.logger.Info(
|
||||||
"reconciled request list",
|
"reconciled request list",
|
||||||
|
|||||||
@@ -453,6 +453,15 @@ func TestReconcileRespectsBatchSize(t *testing.T) {
|
|||||||
f := newReconcileFixture(t)
|
f := newReconcileFixture(t)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// A client that searches and finds nothing. The batch size is about
|
||||||
|
// how many requests one pass *searches for*, which only means
|
||||||
|
// anything when there is something to search with -- a pass with no
|
||||||
|
// provider now attempts nothing at all, deliberately.
|
||||||
|
f.manager.installProvider(
|
||||||
|
Config{ID: 1, Priority: 50},
|
||||||
|
NewFakeProvider(1, "finds-nothing", Caps{CanSearch: true}),
|
||||||
|
)
|
||||||
|
|
||||||
f.reconciler.SetBatch(2)
|
f.reconciler.SetBatch(2)
|
||||||
|
|
||||||
for _, mbid := range []string{"rg-1", "rg-2", "rg-3", "rg-4"} {
|
for _, mbid := range []string{"rg-1", "rg-2", "rg-3", "rg-4"} {
|
||||||
@@ -593,3 +602,72 @@ func TestSummaryReportsNoProviders(t *testing.T) {
|
|||||||
t.Error("summary did not report that no download client is enabled")
|
t.Error("summary did not report that no download client is enabled")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ...and it does not search, which is the part the user sees.
|
||||||
|
//
|
||||||
|
// Attempting with no provider fails every request with "no download
|
||||||
|
// clients are enabled", and RecordAttempt writes that down as an
|
||||||
|
// attempt and schedules a retry -- so a wanted list built deliberately
|
||||||
|
// without a client accrued failures and announced "next check in 6
|
||||||
|
// hours" about a check that cannot happen. Wanting something with no
|
||||||
|
// way to fetch it is supported; being told it is being looked for is
|
||||||
|
// a lie.
|
||||||
|
func TestNoProvidersMeansNoAttempt(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
f := newReconcileFixture(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
id, err := f.store.AddRequest(ctx, Request{
|
||||||
|
MBID: "rg-1",
|
||||||
|
Entity: EntityReleaseGroup,
|
||||||
|
LibraryID: 1,
|
||||||
|
Title: "OK Computer",
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("AddRequest: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
f.catalog.tracklists["rg-1"] = fourTrackDownload().Expected
|
||||||
|
|
||||||
|
summary, err := f.reconciler.RunNow(ctx)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("RunNow: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if summary.Attempted != 0 {
|
||||||
|
t.Errorf("attempted %d requests with no client to search with, want 0",
|
||||||
|
summary.Attempted)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The list still knows what is on it: "nothing happened" has to be
|
||||||
|
// reportable as "nothing was searched for, of the one thing you
|
||||||
|
// want" rather than as silence.
|
||||||
|
if summary.Waiting != 1 {
|
||||||
|
t.Errorf("summary reported %d waiting, want 1", summary.Waiting)
|
||||||
|
}
|
||||||
|
|
||||||
|
req, err := f.store.GetRequest(ctx, id)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetRequest: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if req.Attempts != 0 {
|
||||||
|
t.Errorf("attempts = %d, want 0: a pass that could not search did not",
|
||||||
|
req.Attempts)
|
||||||
|
}
|
||||||
|
|
||||||
|
if req.LastError != "" {
|
||||||
|
t.Errorf("lastError = %q, want empty: the request did not fail, it "+
|
||||||
|
"was never tried", req.LastError)
|
||||||
|
}
|
||||||
|
|
||||||
|
// A new request is due immediately (next_try_at is set to now on
|
||||||
|
// insert), so the fault is not the presence of a time -- it is a
|
||||||
|
// time pushed into the future by a failed attempt, which is what the
|
||||||
|
// UI renders as "next check in 6 hours".
|
||||||
|
if req.NextTryAt.After(time.Now().Add(time.Minute)) {
|
||||||
|
t.Errorf("next try scheduled for %v: a check that cannot happen was "+
|
||||||
|
"put on the clock", req.NextTryAt)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -279,17 +279,19 @@ func (h *MPRISHandler) enqueue(fn func()) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// UpdateMetadata pushes track metadata to D-Bus.
|
// metadataMap builds the org.mpris.MediaPlayer2.Player Metadata value
|
||||||
func (h *MPRISHandler) UpdateMetadata(meta Metadata) {
|
// for one track.
|
||||||
h.mu.Lock()
|
//
|
||||||
h.trackID++
|
// It is separated from UpdateMetadata, which needs a live D-Bus
|
||||||
tid := h.trackID
|
// connection, so the map's contents can be asserted on: this file is
|
||||||
h.mu.Unlock()
|
// behind a build tag and everything in it that touches h is reachable
|
||||||
|
// only from a session bus, which is the same reason the Android
|
||||||
m := map[string]interface{}{
|
// contract lives in an untagged androidpayload.go.
|
||||||
|
func metadataMap(meta Metadata, trackID uint64) map[string]any {
|
||||||
|
m := map[string]any{
|
||||||
"mpris:trackid": dbus.ObjectPath(
|
"mpris:trackid": dbus.ObjectPath(
|
||||||
fmt.Sprintf(
|
fmt.Sprintf(
|
||||||
"/org/yellowjacket/Track/%d", tid,
|
"/org/yellowjacket/Track/%d", trackID,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
}
|
}
|
||||||
@@ -306,16 +308,45 @@ func (h *MPRISHandler) UpdateMetadata(meta Metadata) {
|
|||||||
m["xesam:album"] = meta.Album
|
m["xesam:album"] = meta.Album
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Always present, even with nothing to point at.
|
||||||
|
//
|
||||||
|
// Every other key here can be omitted safely because a client
|
||||||
|
// reading the map sees a track with no title or no album and
|
||||||
|
// renders it that way. Art is different: KDE's applet (and
|
||||||
|
// others) treat an *absent* mpris:artUrl as "no news about the
|
||||||
|
// art" and keep drawing whatever the last track had, so playing
|
||||||
|
// something with no cover left the previous album's sleeve on
|
||||||
|
// screen — which reads as the wrong track playing rather than as
|
||||||
|
// missing artwork.
|
||||||
|
//
|
||||||
|
// An empty string is the honest answer and is what the spec's
|
||||||
|
// "URI" type degrades to; a client that cannot load it falls back
|
||||||
|
// to its own placeholder, which is the behaviour wanted.
|
||||||
|
artURL := ""
|
||||||
if meta.ArtFilePath != "" {
|
if meta.ArtFilePath != "" {
|
||||||
m["mpris:artUrl"] = "file://" + meta.ArtFilePath
|
artURL = "file://" + meta.ArtFilePath
|
||||||
}
|
}
|
||||||
|
|
||||||
|
m["mpris:artUrl"] = artURL
|
||||||
|
|
||||||
if meta.DurationSec > 0 {
|
if meta.DurationSec > 0 {
|
||||||
m["mpris:length"] = int64(
|
m["mpris:length"] = int64(
|
||||||
meta.DurationSec,
|
meta.DurationSec,
|
||||||
) * usPerSec
|
) * usPerSec
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return m
|
||||||
|
}
|
||||||
|
|
||||||
|
// UpdateMetadata pushes track metadata to D-Bus.
|
||||||
|
func (h *MPRISHandler) UpdateMetadata(meta Metadata) {
|
||||||
|
h.mu.Lock()
|
||||||
|
h.trackID++
|
||||||
|
tid := h.trackID
|
||||||
|
h.mu.Unlock()
|
||||||
|
|
||||||
|
m := metadataMap(meta, tid)
|
||||||
|
|
||||||
h.enqueue(func() {
|
h.enqueue(func() {
|
||||||
h.props.SetMust(playerIf, "Metadata", m)
|
h.props.SetMust(playerIf, "Metadata", m)
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -0,0 +1,86 @@
|
|||||||
|
//go:build linux && !android
|
||||||
|
|
||||||
|
package mediacontrols
|
||||||
|
|
||||||
|
import "testing"
|
||||||
|
|
||||||
|
// The one key that must be present even when it is empty.
|
||||||
|
//
|
||||||
|
// Everything else in the map may be omitted, because a client reading
|
||||||
|
// it renders a track with no title as a track with no title. Art is
|
||||||
|
// different: KDE's applet treats an *absent* mpris:artUrl as no news
|
||||||
|
// about the art and keeps drawing the last one it saw, so a track with
|
||||||
|
// no cover wore the previous album's sleeve — which reads as the wrong
|
||||||
|
// track playing rather than as missing artwork.
|
||||||
|
func TestMetadataMapAlwaysCarriesArtURL(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
meta Metadata
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "no art at all",
|
||||||
|
meta: Metadata{Title: "Blue in Green"},
|
||||||
|
want: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "art on disk",
|
||||||
|
meta: Metadata{
|
||||||
|
Title: "Blue in Green",
|
||||||
|
ArtFilePath: "/covers/kind-of-blue_lg.jpg",
|
||||||
|
},
|
||||||
|
want: "file:///covers/kind-of-blue_lg.jpg",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
m := metadataMap(tt.meta, 1)
|
||||||
|
|
||||||
|
got, ok := m["mpris:artUrl"]
|
||||||
|
if !ok {
|
||||||
|
t.Fatal("mpris:artUrl is absent; it must always be sent")
|
||||||
|
}
|
||||||
|
|
||||||
|
if got != tt.want {
|
||||||
|
t.Errorf("mpris:artUrl = %v, want %q", got, tt.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The trackid has to change between tracks or a client is entitled to
|
||||||
|
// treat the metadata as describing the same track it already has.
|
||||||
|
func TestMetadataMapTrackIDVaries(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
first := metadataMap(Metadata{Title: "A"}, 1)["mpris:trackid"]
|
||||||
|
second := metadataMap(Metadata{Title: "B"}, 2)["mpris:trackid"]
|
||||||
|
|
||||||
|
if first == second {
|
||||||
|
t.Errorf("trackid did not change: %v", first)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The optional keys stay optional — this is what makes artUrl's
|
||||||
|
// always-present treatment a deliberate exception rather than drift.
|
||||||
|
func TestMetadataMapOmitsEmptyOptionalFields(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
m := metadataMap(Metadata{}, 1)
|
||||||
|
|
||||||
|
for _, key := range []string{
|
||||||
|
"xesam:title",
|
||||||
|
"xesam:artist",
|
||||||
|
"xesam:album",
|
||||||
|
"mpris:length",
|
||||||
|
} {
|
||||||
|
if _, ok := m[key]; ok {
|
||||||
|
t.Errorf("%s is present for an empty Metadata", key)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -81,6 +81,7 @@ func (q *Queue) emitTracksModified(
|
|||||||
Index: index,
|
Index: index,
|
||||||
Positions: positions,
|
Positions: positions,
|
||||||
CurrentIndex: q.currentIndex,
|
CurrentIndex: q.currentIndex,
|
||||||
|
Source: q.source,
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -219,6 +219,56 @@ func TestEmit_AddTrackSendsDeltaNotSnapshot(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The append clears the source, and the delta is the only event those
|
||||||
|
// paths emit — so if it does not carry the source, the frontend keeps
|
||||||
|
// the label it was last given and goes on offering a link back to an
|
||||||
|
// album the queue no longer holds until something forces a full state.
|
||||||
|
func TestEmit_AppendDeltaCarriesClearedSource(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
q, db, rec := setupRecordedQueue(t)
|
||||||
|
paths := seedAudioFiles(t, db, 4)
|
||||||
|
|
||||||
|
q.SetQueue(
|
||||||
|
paths[:3], 0, false,
|
||||||
|
Source{Type: "album", ID: 1, Label: "Abbey Road"},
|
||||||
|
)
|
||||||
|
|
||||||
|
if _, ok := rec.Wait(events.QueueChanged, waitFor); !ok {
|
||||||
|
t.Fatalf("no QueueChanged after SetQueue; got %v", rec.Names())
|
||||||
|
}
|
||||||
|
|
||||||
|
rec.Reset()
|
||||||
|
q.AddTrack(paths[3])
|
||||||
|
|
||||||
|
if got := modifiedOf(t, rec).Source; got != (Source{}) {
|
||||||
|
t.Errorf("delta source = %+v, want zero value", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// And a delta that did not clear it still reports the source it has,
|
||||||
|
// or the frontend would drop a perfectly good label on every removal.
|
||||||
|
func TestEmit_NonAppendDeltaCarriesSource(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
q, db, rec := setupRecordedQueue(t)
|
||||||
|
paths := seedAudioFiles(t, db, 4)
|
||||||
|
|
||||||
|
album := Source{Type: "album", ID: 1, Label: "Abbey Road"}
|
||||||
|
q.SetQueue(paths, 0, false, album)
|
||||||
|
|
||||||
|
if _, ok := rec.Wait(events.QueueChanged, waitFor); !ok {
|
||||||
|
t.Fatalf("no QueueChanged after SetQueue; got %v", rec.Names())
|
||||||
|
}
|
||||||
|
|
||||||
|
rec.Reset()
|
||||||
|
q.RemoveTrack(3)
|
||||||
|
|
||||||
|
if got := modifiedOf(t, rec).Source; got != album {
|
||||||
|
t.Errorf("delta source = %+v, want %+v", got, album)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestEmit_RemoveTracksReportsPositions(t *testing.T) {
|
func TestEmit_RemoveTracksReportsPositions(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -156,12 +156,21 @@ type PlaybackFailure struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// TracksModified is the payload for the QueueTracksModified event.
|
// TracksModified is the payload for the QueueTracksModified event.
|
||||||
|
//
|
||||||
|
// Source is carried because an append is exactly what can *invalidate*
|
||||||
|
// it: a queue built from one album stops being that album the moment a
|
||||||
|
// track from somewhere else is added to it. The delta is the only event
|
||||||
|
// those paths emit, so without this the frontend would keep the label
|
||||||
|
// it was last given and go on saying "Playing from" an album that is no
|
||||||
|
// longer what is queued — an event carrying what its consumer needs, so
|
||||||
|
// nothing has to invalidate anything.
|
||||||
type TracksModified struct {
|
type TracksModified struct {
|
||||||
Action string `json:"action"`
|
Action string `json:"action"`
|
||||||
Tracks []Track `json:"tracks,omitempty"`
|
Tracks []Track `json:"tracks,omitempty"`
|
||||||
Index int `json:"index"`
|
Index int `json:"index"`
|
||||||
Positions []int `json:"positions,omitempty"`
|
Positions []int `json:"positions,omitempty"`
|
||||||
CurrentIndex int `json:"currentIndex"`
|
CurrentIndex int `json:"currentIndex"`
|
||||||
|
Source Source `json:"source"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// Queue manages an ordered list of tracks for playback.
|
// Queue manages an ordered list of tracks for playback.
|
||||||
@@ -455,6 +464,8 @@ func (q *Queue) AddTrack(filePath string) {
|
|||||||
q.generateShuffleOrder()
|
q.generateShuffleOrder()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
q.dropSource()
|
||||||
|
|
||||||
q.persistAddTrack(track)
|
q.persistAddTrack(track)
|
||||||
q.persistState()
|
q.persistState()
|
||||||
q.emitTracksModified(
|
q.emitTracksModified(
|
||||||
@@ -505,6 +516,8 @@ func (q *Queue) AddTracks(filePaths []string) {
|
|||||||
q.generateShuffleOrder()
|
q.generateShuffleOrder()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
q.dropSource()
|
||||||
|
|
||||||
q.persistAddTracks(newTracks)
|
q.persistAddTracks(newTracks)
|
||||||
q.persistState()
|
q.persistState()
|
||||||
q.emitTracksModified(
|
q.emitTracksModified(
|
||||||
@@ -563,6 +576,8 @@ func (q *Queue) InsertNextTracks(filePaths []string) {
|
|||||||
q.generateShuffleOrder()
|
q.generateShuffleOrder()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
q.dropSource()
|
||||||
|
|
||||||
q.persistInsertTracks(newTracks, insertPos)
|
q.persistInsertTracks(newTracks, insertPos)
|
||||||
q.persistState()
|
q.persistState()
|
||||||
q.emitTracksModified(
|
q.emitTracksModified(
|
||||||
@@ -613,6 +628,8 @@ func (q *Queue) InsertNext(filePath string) {
|
|||||||
q.generateShuffleOrder()
|
q.generateShuffleOrder()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
q.dropSource()
|
||||||
|
|
||||||
q.persistInsertTracks([]Track{track}, insertPos)
|
q.persistInsertTracks([]Track{track}, insertPos)
|
||||||
q.persistState()
|
q.persistState()
|
||||||
q.emitTracksModified(
|
q.emitTracksModified(
|
||||||
@@ -680,6 +697,8 @@ func (q *Queue) InsertTracksAt(filePaths []string, index int) {
|
|||||||
q.generateShuffleOrder()
|
q.generateShuffleOrder()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
q.dropSource()
|
||||||
|
|
||||||
q.persistInsertTracks(newTracks, index)
|
q.persistInsertTracks(newTracks, index)
|
||||||
q.persistState()
|
q.persistState()
|
||||||
q.emitTracksModified(
|
q.emitTracksModified(
|
||||||
@@ -1537,6 +1556,31 @@ func (q *Queue) reindexPositions() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// dropSource forgets which collection the queue was built from.
|
||||||
|
//
|
||||||
|
// A Source is a claim that everything queued came from one album,
|
||||||
|
// playlist, genre or artist, and the frontend renders it as a
|
||||||
|
// "Playing from X" link back to that page. Adding or inserting a track
|
||||||
|
// makes the claim false — the queue is now that album *plus* something
|
||||||
|
// else — so every path that does so calls this.
|
||||||
|
//
|
||||||
|
// It was set by SetQueue and cleared in exactly one place, Clear, so a
|
||||||
|
// label survived every append. It is persisted too (source_type /
|
||||||
|
// source_id / source_label on the queue state row), which is what made
|
||||||
|
// a wrong label outlive the session that earned it: an album queued on
|
||||||
|
// Monday, added to on Tuesday, still offered a link back to that album
|
||||||
|
// on Friday.
|
||||||
|
//
|
||||||
|
// Removing, reordering and shuffling deliberately do not call this. A
|
||||||
|
// queue with a track taken out of it, or played in another order, is
|
||||||
|
// still that album — the link still goes somewhere true. Only the
|
||||||
|
// arrival of a track from elsewhere makes it a lie.
|
||||||
|
//
|
||||||
|
// The caller must hold q.mu.
|
||||||
|
func (q *Queue) dropSource() {
|
||||||
|
q.source = Source{}
|
||||||
|
}
|
||||||
|
|
||||||
// commitMutation persists the current queue state after a mutation.
|
// commitMutation persists the current queue state after a mutation.
|
||||||
// When reindex is true, track positions are renumbered first.
|
// When reindex is true, track positions are renumbered first.
|
||||||
// The caller must hold q.mu.
|
// The caller must hold q.mu.
|
||||||
|
|||||||
@@ -126,6 +126,117 @@ func TestClear_ResetsSource(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A queue built from one album stops being that album the moment a
|
||||||
|
// track from somewhere else joins it, so every path that adds one
|
||||||
|
// drops the source. Before this, SetQueue was the only writer and
|
||||||
|
// Clear the only clearer, so "Playing from Abbey Road" outlived every
|
||||||
|
// append — and, being persisted, every restart too.
|
||||||
|
func TestAppendPathsDropSource(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
album := Source{Type: "album", ID: 1, Label: "Abbey Road"}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
append func(q *Queue, paths []string)
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "AddTrack",
|
||||||
|
append: func(q *Queue, paths []string) {
|
||||||
|
q.AddTrack(paths[5])
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "AddTracks",
|
||||||
|
append: func(q *Queue, paths []string) {
|
||||||
|
q.AddTracks(paths[5:7])
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "InsertNext",
|
||||||
|
append: func(q *Queue, paths []string) {
|
||||||
|
q.InsertNext(paths[5])
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "InsertNextTracks",
|
||||||
|
append: func(q *Queue, paths []string) {
|
||||||
|
q.InsertNextTracks(paths[5:7])
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "InsertTracksAt",
|
||||||
|
append: func(q *Queue, paths []string) {
|
||||||
|
q.InsertTracksAt(paths[5:7], 1)
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
q, db := setupTestQueue(t)
|
||||||
|
paths := seedAudioFiles(t, db, 8)
|
||||||
|
|
||||||
|
q.SetQueue(paths[:5], 0, false, album)
|
||||||
|
|
||||||
|
if got := q.GetState().Source; got != album {
|
||||||
|
t.Fatalf("source before append: got %+v, want %+v", got, album)
|
||||||
|
}
|
||||||
|
|
||||||
|
tt.append(q, paths)
|
||||||
|
|
||||||
|
if got := q.GetState().Source; got != (Source{}) {
|
||||||
|
t.Errorf(
|
||||||
|
"source after %s: got %+v, want zero value",
|
||||||
|
tt.name, got,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Removing and reordering deliberately do not drop it: a queue with a
|
||||||
|
// track taken out of it is still that album, and the link still goes
|
||||||
|
// somewhere true.
|
||||||
|
func TestRemoveAndMoveKeepSource(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
album := Source{Type: "album", ID: 1, Label: "Abbey Road"}
|
||||||
|
|
||||||
|
t.Run("RemoveTrack", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
q, db := setupTestQueue(t)
|
||||||
|
paths := seedAudioFiles(t, db, 5)
|
||||||
|
|
||||||
|
q.SetQueue(paths, 0, false, album)
|
||||||
|
q.RemoveTrack(3)
|
||||||
|
|
||||||
|
if got := q.GetState().Source; got != album {
|
||||||
|
t.Errorf("source after RemoveTrack: got %+v, want %+v", got, album)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("MoveQueueTracks", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
q, db := setupTestQueue(t)
|
||||||
|
paths := seedAudioFiles(t, db, 5)
|
||||||
|
|
||||||
|
q.SetQueue(paths, 0, false, album)
|
||||||
|
q.MoveQueueTracks([]int{0}, 3)
|
||||||
|
|
||||||
|
if got := q.GetState().Source; got != album {
|
||||||
|
t.Errorf(
|
||||||
|
"source after MoveQueueTracks: got %+v, want %+v",
|
||||||
|
got, album,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
func TestSetQueue_WithStartIndex(t *testing.T) {
|
func TestSetQueue_WithStartIndex(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ var (
|
|||||||
errUnsupportedOp = errors.New("unsupported operator")
|
errUnsupportedOp = errors.New("unsupported operator")
|
||||||
errInvalidSortField = errors.New("invalid sort field: not in allowed field list")
|
errInvalidSortField = errors.New("invalid sort field: not in allowed field list")
|
||||||
errNotNumeric = errors.New("value must be numeric")
|
errNotNumeric = errors.New("value must be numeric")
|
||||||
|
errInvalidMatch = errors.New("match must be \"all\" or \"any\"")
|
||||||
)
|
)
|
||||||
|
|
||||||
// Rule represents a single filter condition for a smart playlist.
|
// Rule represents a single filter condition for a smart playlist.
|
||||||
@@ -37,13 +38,45 @@ type Rule struct {
|
|||||||
Value string `json:"value"`
|
Value string `json:"value"`
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// MatchType decides how a rule set's conditions combine.
|
||||||
|
//
|
||||||
|
// The rules used to be joined with " AND " and nothing else, so a
|
||||||
|
// playlist could only ever narrow: "jazz released after 1960" was
|
||||||
|
// expressible and "jazz or blues" was not, which is most of what
|
||||||
|
// anyone reaches for a second rule to say.
|
||||||
|
type MatchType string
|
||||||
|
|
||||||
|
const (
|
||||||
|
// MatchAll requires every rule to hold — the historical behaviour,
|
||||||
|
// and what an empty match means so that every rule set written
|
||||||
|
// before this existed keeps the meaning it was saved with.
|
||||||
|
MatchAll MatchType = "all"
|
||||||
|
// MatchAny requires at least one rule to hold.
|
||||||
|
MatchAny MatchType = "any"
|
||||||
|
)
|
||||||
|
|
||||||
|
// joiner returns the SQL keyword that combines two conditions.
|
||||||
|
// An unrecognised value cannot reach here — ParseRuleSet rejects one
|
||||||
|
// — so the default is about the empty string, which is every rule set
|
||||||
|
// saved before this field existed.
|
||||||
|
func (m MatchType) joiner() string {
|
||||||
|
if m == MatchAny {
|
||||||
|
return " OR "
|
||||||
|
}
|
||||||
|
|
||||||
|
return " AND "
|
||||||
|
}
|
||||||
|
|
||||||
// RuleSet holds the complete filter configuration for a smart
|
// RuleSet holds the complete filter configuration for a smart
|
||||||
// playlist, including optional sort and limit.
|
// playlist, including optional sort and limit.
|
||||||
type RuleSet struct {
|
type RuleSet struct {
|
||||||
Rules []Rule `json:"rules"`
|
Rules []Rule `json:"rules"`
|
||||||
Limit int `json:"limit,omitempty"`
|
// Match is "all" or "any"; empty means "all". It is omitempty so
|
||||||
SortField string `json:"sort_field,omitempty"`
|
// an untouched playlist's stored JSON does not change shape.
|
||||||
SortDir string `json:"sort_dir,omitempty"`
|
Match MatchType `json:"match,omitempty"`
|
||||||
|
Limit int `json:"limit,omitempty"`
|
||||||
|
SortField string `json:"sort_field,omitempty"`
|
||||||
|
SortDir string `json:"sort_dir,omitempty"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// fieldMap maps user-facing rule field names to track_metadata column
|
// fieldMap maps user-facing rule field names to track_metadata column
|
||||||
@@ -116,7 +149,12 @@ const genreDelimiter = "||"
|
|||||||
// slice of rules. It is a pure function — no database access needed.
|
// slice of rules. It is a pure function — no database access needed.
|
||||||
// Returns the clause (without the leading "WHERE"), the parameter
|
// Returns the clause (without the leading "WHERE"), the parameter
|
||||||
// args, and any validation error.
|
// args, and any validation error.
|
||||||
func BuildWhereClause(rules []Rule) (string, []any, error) {
|
//
|
||||||
|
// match decides how the conditions combine; an empty match is MatchAll,
|
||||||
|
// which is what every rule set saved before the field existed means.
|
||||||
|
func BuildWhereClause(
|
||||||
|
rules []Rule, match MatchType,
|
||||||
|
) (string, []any, error) {
|
||||||
if len(rules) == 0 {
|
if len(rules) == 0 {
|
||||||
return "", nil, nil
|
return "", nil, nil
|
||||||
}
|
}
|
||||||
@@ -179,7 +217,28 @@ func BuildWhereClause(rules []Rule) (string, []any, error) {
|
|||||||
args = append(args, condArgs...)
|
args = append(args, condArgs...)
|
||||||
}
|
}
|
||||||
|
|
||||||
return strings.Join(conditions, " AND "), args, nil
|
// Under OR, each condition is parenthesised; under AND it is not.
|
||||||
|
//
|
||||||
|
// The asymmetry is deliberate rather than an omission. AND is the
|
||||||
|
// tighter operator in SQL, so an OR-join has to protect any
|
||||||
|
// condition that contains a top-level AND of its own or the halves
|
||||||
|
// come apart: `days_since_played less_than` is
|
||||||
|
// `last_played IS NOT NULL AND <expr> < ?`, which read without
|
||||||
|
// brackets under an OR-join happens to still parse correctly and
|
||||||
|
// would stop doing so the moment a condition grows a top-level OR.
|
||||||
|
// Bracketing under AND would be a no-op semantically and would
|
||||||
|
// rewrite the clause every existing test pins, so the brackets go
|
||||||
|
// exactly where they change something.
|
||||||
|
if match == MatchAny {
|
||||||
|
bracketed := make([]string, len(conditions))
|
||||||
|
for i, cond := range conditions {
|
||||||
|
bracketed[i] = "(" + cond + ")"
|
||||||
|
}
|
||||||
|
|
||||||
|
conditions = bracketed
|
||||||
|
}
|
||||||
|
|
||||||
|
return strings.Join(conditions, match.joiner()), args, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// validateOperator checks that the operator is valid for the field
|
// validateOperator checks that the operator is valid for the field
|
||||||
@@ -599,7 +658,7 @@ func Evaluate(
|
|||||||
start := time.Now()
|
start := time.Now()
|
||||||
logger := db.Logger()
|
logger := db.Logger()
|
||||||
|
|
||||||
where, args, err := BuildWhereClause(ruleSet.Rules)
|
where, args, err := BuildWhereClause(ruleSet.Rules, ruleSet.Match)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf(
|
return nil, fmt.Errorf(
|
||||||
"smart playlist rule error: %w", err,
|
"smart playlist rule error: %w", err,
|
||||||
@@ -1036,6 +1095,16 @@ func ParseRuleSet(jsonStr string) (RuleSet, error) {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A match nobody recognises would otherwise fall through to AND,
|
||||||
|
// which is a playlist quietly returning the wrong tracks rather
|
||||||
|
// than refusing to be saved. This is the only place a rule set
|
||||||
|
// enters the backend, so it is the only place that has to ask.
|
||||||
|
if rs.Match != "" && rs.Match != MatchAll && rs.Match != MatchAny {
|
||||||
|
return RuleSet{}, fmt.Errorf(
|
||||||
|
"%w: %q", errInvalidMatch, rs.Match,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
return rs, nil
|
return rs, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package smartplaylist
|
package smartplaylist
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
@@ -170,7 +171,7 @@ func TestBuildWhereClause_TextIs(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "artist", Operator: "is", Value: "Queen"},
|
{Field: "artist", Operator: "is", Value: "Queen"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -189,7 +190,7 @@ func TestBuildWhereClause_TextIsNot(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "artist", Operator: "is_not", Value: "Queen"},
|
{Field: "artist", Operator: "is_not", Value: "Queen"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -209,7 +210,7 @@ func TestBuildWhereClause_TextContains(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "title", Operator: "contains", Value: "Black"},
|
{Field: "title", Operator: "contains", Value: "Black"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -231,7 +232,7 @@ func TestBuildWhereClause_TextDoesNotContain(t *testing.T) {
|
|||||||
Field: "title", Operator: "does_not_contain",
|
Field: "title", Operator: "does_not_contain",
|
||||||
Value: "Black",
|
Value: "Black",
|
||||||
},
|
},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -251,7 +252,7 @@ func TestBuildWhereClause_TextStartsWith(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "title", Operator: "starts_with", Value: "Back"},
|
{Field: "title", Operator: "starts_with", Value: "Back"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -270,7 +271,7 @@ func TestBuildWhereClause_TextEndsWith(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "title", Operator: "ends_with", Value: "Black"},
|
{Field: "title", Operator: "ends_with", Value: "Black"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -292,7 +293,7 @@ func TestBuildWhereClause_TextIsAnyOf(t *testing.T) {
|
|||||||
Field: "artist", Operator: "is_any_of",
|
Field: "artist", Operator: "is_any_of",
|
||||||
Value: `["Queen","AC/DC"]`,
|
Value: `["Queen","AC/DC"]`,
|
||||||
},
|
},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -312,7 +313,7 @@ func TestBuildWhereClause_NumericIs(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "year", Operator: "is", Value: "1980"},
|
{Field: "year", Operator: "is", Value: "1980"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -331,7 +332,7 @@ func TestBuildWhereClause_NumericIsNot(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "year", Operator: "is_not", Value: "1980"},
|
{Field: "year", Operator: "is_not", Value: "1980"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -350,7 +351,7 @@ func TestBuildWhereClause_NumericGreaterThan(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "year", Operator: "greater_than", Value: "2000"},
|
{Field: "year", Operator: "greater_than", Value: "2000"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -369,7 +370,7 @@ func TestBuildWhereClause_NumericLessThan(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "year", Operator: "less_than", Value: "1980"},
|
{Field: "year", Operator: "less_than", Value: "1980"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -391,7 +392,7 @@ func TestBuildWhereClause_NumericBetween(t *testing.T) {
|
|||||||
Field: "year", Operator: "between",
|
Field: "year", Operator: "between",
|
||||||
Value: "1975,1985",
|
Value: "1975,1985",
|
||||||
},
|
},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -414,7 +415,7 @@ func TestBuildWhereClause_NumericBetweenJSON(t *testing.T) {
|
|||||||
Field: "year", Operator: "between",
|
Field: "year", Operator: "between",
|
||||||
Value: `["1975","1985"]`,
|
Value: `["1975","1985"]`,
|
||||||
},
|
},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -434,7 +435,7 @@ func TestBuildWhereClause_GenreIsProducesSubquery(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "genre", Operator: "is", Value: "Rock"},
|
{Field: "genre", Operator: "is", Value: "Rock"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -466,7 +467,7 @@ func TestBuildWhereClause_GenreIsNotProducesSubquery(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "genre", Operator: "is_not", Value: "Rock"},
|
{Field: "genre", Operator: "is_not", Value: "Rock"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -495,7 +496,7 @@ func TestBuildWhereClause_GenreIsAnyOfProducesSubquery(t *testing.T) {
|
|||||||
Field: "genre", Operator: "is_any_of",
|
Field: "genre", Operator: "is_any_of",
|
||||||
Value: `["Rock","Pop"]`,
|
Value: `["Rock","Pop"]`,
|
||||||
},
|
},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -524,7 +525,7 @@ func TestBuildWhereClause_GenreContainsUsesSubquery(t *testing.T) {
|
|||||||
|
|
||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "genre", Operator: "contains", Value: "Rock"},
|
{Field: "genre", Operator: "contains", Value: "Rock"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -557,7 +558,7 @@ func TestBuildWhereClause_MultipleRulesAND(t *testing.T) {
|
|||||||
clause, args, err := BuildWhereClause([]Rule{
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
{Field: "artist", Operator: "is", Value: "Queen"},
|
{Field: "artist", Operator: "is", Value: "Queen"},
|
||||||
{Field: "year", Operator: "greater_than", Value: "1975"},
|
{Field: "year", Operator: "greater_than", Value: "1975"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -572,6 +573,110 @@ func TestBuildWhereClause_MultipleRulesAND(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestBuildWhereClause_MultipleRulesOR(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
clause, args, err := BuildWhereClause([]Rule{
|
||||||
|
{Field: "artist", Operator: "is", Value: "Queen"},
|
||||||
|
{Field: "year", Operator: "greater_than", Value: "1975"},
|
||||||
|
}, MatchAny)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
want := "(artist_name = ? COLLATE NOCASE) OR (year > ?)"
|
||||||
|
if clause != want {
|
||||||
|
t.Errorf("clause = %q, want %q", clause, want)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(args) != 2 || args[0] != "Queen" || args[1] != int64(1975) {
|
||||||
|
t.Errorf("args = %v, want [Queen 1975]", args)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// An empty match is what every rule set saved before the field existed
|
||||||
|
// carries, and it has to keep meaning AND — a playlist silently
|
||||||
|
// widening to OR on upgrade is the whole risk of adding this field.
|
||||||
|
func TestBuildWhereClause_EmptyMatchIsAll(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
rules := []Rule{
|
||||||
|
{Field: "artist", Operator: "is", Value: "Queen"},
|
||||||
|
{Field: "year", Operator: "greater_than", Value: "1975"},
|
||||||
|
}
|
||||||
|
|
||||||
|
empty, _, err := BuildWhereClause(rules, "")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
all, _, err := BuildWhereClause(rules, MatchAll)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if empty != all {
|
||||||
|
t.Errorf("empty match = %q, want the same as MatchAll %q",
|
||||||
|
empty, all)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A condition carrying its own top-level AND is what makes the
|
||||||
|
// bracketing under OR load-bearing: `days_since_played less_than`
|
||||||
|
// is two predicates, and both belong to the same rule.
|
||||||
|
func TestBuildWhereClause_ORBracketsCompoundCondition(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
clause, _, err := BuildWhereClause([]Rule{
|
||||||
|
{Field: "artist", Operator: "is", Value: "Queen"},
|
||||||
|
{
|
||||||
|
Field: "days_since_played",
|
||||||
|
Operator: "less_than",
|
||||||
|
Value: "30",
|
||||||
|
},
|
||||||
|
}, MatchAny)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if !strings.Contains(clause, "(last_played IS NOT NULL AND") {
|
||||||
|
t.Errorf(
|
||||||
|
"compound condition is not bracketed under OR: %q",
|
||||||
|
clause,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestParseRuleSet_RejectsUnknownMatch(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
_, err := ParseRuleSet(`{"rules":[],"match":"either"}`)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected an error for an unknown match type")
|
||||||
|
}
|
||||||
|
|
||||||
|
if !errors.Is(err, errInvalidMatch) {
|
||||||
|
t.Errorf("err = %v, want errInvalidMatch", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestParseRuleSet_AcceptsAnyAndAll(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
for _, want := range []MatchType{MatchAll, MatchAny} {
|
||||||
|
rs, err := ParseRuleSet(
|
||||||
|
`{"rules":[],"match":"` + string(want) + `"}`,
|
||||||
|
)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("match %q: unexpected error: %v", want, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if rs.Match != want {
|
||||||
|
t.Errorf("match = %q, want %q", rs.Match, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
|
func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
@@ -581,7 +686,7 @@ func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
|
|||||||
Field: "genre", Operator: "does_not_contain",
|
Field: "genre", Operator: "does_not_contain",
|
||||||
Value: "Punk",
|
Value: "Punk",
|
||||||
},
|
},
|
||||||
})
|
}, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -609,7 +714,7 @@ func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
|
|||||||
func TestBuildWhereClause_EmptyRules(t *testing.T) {
|
func TestBuildWhereClause_EmptyRules(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
clause, args, err := BuildWhereClause(nil)
|
clause, args, err := BuildWhereClause(nil, MatchAll)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unexpected error: %v", err)
|
t.Fatalf("unexpected error: %v", err)
|
||||||
}
|
}
|
||||||
@@ -631,7 +736,7 @@ func TestBuildWhereClause_InvalidField(t *testing.T) {
|
|||||||
Field: "nonexistent", Operator: "is",
|
Field: "nonexistent", Operator: "is",
|
||||||
Value: "anything",
|
Value: "anything",
|
||||||
},
|
},
|
||||||
})
|
}, MatchAll)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal("expected error for invalid field, got nil")
|
t.Fatal("expected error for invalid field, got nil")
|
||||||
}
|
}
|
||||||
@@ -654,7 +759,7 @@ func TestBuildWhereClause_InvalidOperatorForNumeric(t *testing.T) {
|
|||||||
|
|
||||||
_, _, err := BuildWhereClause([]Rule{
|
_, _, err := BuildWhereClause([]Rule{
|
||||||
{Field: "year", Operator: "contains", Value: "1980"},
|
{Field: "year", Operator: "contains", Value: "1980"},
|
||||||
})
|
}, MatchAll)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal(
|
t.Fatal(
|
||||||
"expected error for text operator on numeric field",
|
"expected error for text operator on numeric field",
|
||||||
@@ -676,7 +781,7 @@ func TestBuildWhereClause_InvalidOperatorForText(t *testing.T) {
|
|||||||
Field: "artist", Operator: "greater_than",
|
Field: "artist", Operator: "greater_than",
|
||||||
Value: "Queen",
|
Value: "Queen",
|
||||||
},
|
},
|
||||||
})
|
}, MatchAll)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal(
|
t.Fatal(
|
||||||
"expected error for numeric operator on text field",
|
"expected error for numeric operator on text field",
|
||||||
@@ -723,6 +828,80 @@ func TestEvaluate_TextIs(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Two rules that share no track at all: under AND this is empty, and
|
||||||
|
// under OR it is the union. Before Match existed only the first was
|
||||||
|
// expressible, so a playlist could only ever narrow — "jazz or blues"
|
||||||
|
// had no way to be said.
|
||||||
|
func TestEvaluate_MatchAnyUnionsWhereMatchAllIntersects(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
db := database.NewTestDB(t)
|
||||||
|
seedSmartPlaylistData(t, db)
|
||||||
|
|
||||||
|
// Queen has two tracks; Beyoncé has one; no track is by both.
|
||||||
|
rules := []Rule{
|
||||||
|
{Field: "artist", Operator: "is", Value: "Queen"},
|
||||||
|
{Field: "artist", Operator: "is", Value: "Beyoncé"},
|
||||||
|
}
|
||||||
|
|
||||||
|
all, err := Evaluate(db, RuleSet{Rules: rules, Match: MatchAll})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Evaluate(all): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(all) != 0 {
|
||||||
|
t.Errorf("match=all returned %d tracks, want 0", len(all))
|
||||||
|
}
|
||||||
|
|
||||||
|
either, err := Evaluate(db, RuleSet{Rules: rules, Match: MatchAny})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Evaluate(any): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(either) != 3 {
|
||||||
|
t.Fatalf("match=any returned %d tracks, want 3", len(either))
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tr := range either {
|
||||||
|
if tr.ArtistName != "Queen" && tr.ArtistName != "Beyoncé" {
|
||||||
|
t.Errorf(
|
||||||
|
"track %q has artist %q, want Queen or Beyoncé",
|
||||||
|
tr.TrackName, tr.ArtistName,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// An empty match is what every playlist saved before the field existed
|
||||||
|
// carries, and it has to keep meaning AND all the way through Evaluate
|
||||||
|
// — a stored playlist silently widening on upgrade is the only real
|
||||||
|
// risk in adding this.
|
||||||
|
func TestEvaluate_EmptyMatchStillIntersects(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
db := database.NewTestDB(t)
|
||||||
|
seedSmartPlaylistData(t, db)
|
||||||
|
|
||||||
|
tracks, err := Evaluate(db, RuleSet{
|
||||||
|
Rules: []Rule{
|
||||||
|
{Field: "artist", Operator: "is", Value: "Queen"},
|
||||||
|
{Field: "year", Operator: "greater_than", Value: "1979"},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Evaluate: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Only "Another One Bites the Dust" (Queen, 1980) satisfies both.
|
||||||
|
if len(tracks) != 1 {
|
||||||
|
t.Fatalf("got %d tracks, want 1", len(tracks))
|
||||||
|
}
|
||||||
|
|
||||||
|
if want := "Another One Bites the Dust"; tracks[0].TrackName != want {
|
||||||
|
t.Errorf("got %q, want %q", tracks[0].TrackName, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestEvaluate_ArtworkEnrichment verifies the presentation-only
|
// TestEvaluate_ArtworkEnrichment verifies the presentation-only
|
||||||
// cover-art and MusicBrainz-ID fields are attached to matched tracks
|
// cover-art and MusicBrainz-ID fields are attached to matched tracks
|
||||||
// by the batched fetchArtwork pass (they are no longer part of the
|
// by the batched fetchArtwork pass (they are no longer part of the
|
||||||
@@ -1340,7 +1519,7 @@ func TestSQLInjection_FieldName(t *testing.T) {
|
|||||||
Field: "title; DROP TABLE playlists",
|
Field: "title; DROP TABLE playlists",
|
||||||
Operator: "is", Value: "x",
|
Operator: "is", Value: "x",
|
||||||
},
|
},
|
||||||
})
|
}, MatchAll)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal(
|
t.Fatal(
|
||||||
"expected error for injected field name, got nil",
|
"expected error for injected field name, got nil",
|
||||||
|
|||||||
@@ -0,0 +1,66 @@
|
|||||||
|
import { test, expect } from '../support/fixtures.js';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The queue button says whether the queue is open.
|
||||||
|
*
|
||||||
|
* It used to look identical in both states, so the only way to tell
|
||||||
|
* what pressing it would do was to look at the other side of the window
|
||||||
|
* and infer it — and for anyone not looking at all there was nothing to
|
||||||
|
* infer from: no `aria-expanded`, no `aria-controls`, no pressed state.
|
||||||
|
*
|
||||||
|
* The state is reflected *from the panel*, not kept beside the click,
|
||||||
|
* because the button is not the only thing that opens the queue —
|
||||||
|
* `now-playing-view` sets the same attribute, since it hides the bar
|
||||||
|
* this button lives in. A flag maintained by the click handler would be
|
||||||
|
* right until something else opened the panel and then quietly wrong,
|
||||||
|
* which is the second test here.
|
||||||
|
*/
|
||||||
|
test.describe('the queue toggle', () => {
|
||||||
|
test('reports open and closed, and names what it controls', async ({
|
||||||
|
app,
|
||||||
|
}) => {
|
||||||
|
const toggle = app.locator('#queue-button');
|
||||||
|
|
||||||
|
await expect(toggle).toHaveAttribute('aria-controls', 'queue-panel');
|
||||||
|
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
|
||||||
|
|
||||||
|
await toggle.click();
|
||||||
|
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
|
||||||
|
|
||||||
|
// The state is not only in the accessibility tree: a control that
|
||||||
|
// announces a state it does not draw is half a fix.
|
||||||
|
//
|
||||||
|
// Background rather than colour, because the pointer is still on
|
||||||
|
// the button after the click and `:hover` paints it the same accent
|
||||||
|
// the open state does -- so a colour comparison here passes on the
|
||||||
|
// broken build and proves nothing.
|
||||||
|
const [open, closed] = await toggle.evaluate((el) => {
|
||||||
|
const now = getComputedStyle(el).backgroundColor;
|
||||||
|
|
||||||
|
el.setAttribute('aria-expanded', 'false');
|
||||||
|
const shut = getComputedStyle(el).backgroundColor;
|
||||||
|
|
||||||
|
el.setAttribute('aria-expanded', 'true');
|
||||||
|
|
||||||
|
return [now, shut];
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(open).not.toBe(closed);
|
||||||
|
|
||||||
|
await toggle.click();
|
||||||
|
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('follows the panel when something else opens it', async ({ app }) => {
|
||||||
|
const toggle = app.locator('#queue-button');
|
||||||
|
|
||||||
|
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
|
||||||
|
|
||||||
|
// Exactly what `now-playing-view`'s queue button does.
|
||||||
|
await app.evaluate(() =>
|
||||||
|
document.getElementById('queue-panel')?.setAttribute('open', ''),
|
||||||
|
);
|
||||||
|
|
||||||
|
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -207,6 +207,17 @@ body div.sidebar {
|
|||||||
color: var(--yj-accent, #ffd43b);
|
color: var(--yj-accent, #ffd43b);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* An open queue is a state this button can be in, and it used to
|
||||||
|
look exactly like the closed one -- so the only way to tell what
|
||||||
|
pressing it would do was to look at the other side of the window
|
||||||
|
and infer it. `aria-expanded` is the same fact for anyone not
|
||||||
|
looking at all, and it points at the panel it controls. */
|
||||||
|
#queue-button[aria-expanded='true'] {
|
||||||
|
color: var(--yj-accent, #ffd43b);
|
||||||
|
background: var(--yj-bg-overlay, #404040);
|
||||||
|
border-radius: 4px;
|
||||||
|
}
|
||||||
|
|
||||||
#queue-button.drag-over {
|
#queue-button.drag-over {
|
||||||
color: var(--yj-accent, #ffd43b);
|
color: var(--yj-accent, #ffd43b);
|
||||||
outline: 2px dashed var(--yj-accent, #ffd43b);
|
outline: 2px dashed var(--yj-accent, #ffd43b);
|
||||||
|
|||||||
+2
-1
@@ -37,7 +37,8 @@
|
|||||||
<footer class="bottom-bar">
|
<footer class="bottom-bar">
|
||||||
<now-playing></now-playing>
|
<now-playing></now-playing>
|
||||||
<audio-player></audio-player>
|
<audio-player></audio-player>
|
||||||
<button aria-label="Toggle queue" id="queue-button">
|
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
|
||||||
|
id="queue-button">
|
||||||
<wa-icon name="list"></wa-icon>
|
<wa-icon name="list"></wa-icon>
|
||||||
</button>
|
</button>
|
||||||
</footer>
|
</footer>
|
||||||
|
|||||||
@@ -521,6 +521,28 @@ if (queueButton && queuePanel) {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The button says whether the panel is open, and it learns that
|
||||||
|
// from the panel rather than from its own click handler.
|
||||||
|
//
|
||||||
|
// It is not the only thing that opens the queue -- `now-playing-view`
|
||||||
|
// sets the same attribute, because it hides the bar this button
|
||||||
|
// lives in -- so a state kept beside the click would be right until
|
||||||
|
// something else opened the panel and then quietly wrong. The panel's
|
||||||
|
// `open` attribute is the one fact; this reflects it.
|
||||||
|
const reflectQueueState = () => {
|
||||||
|
queueButton.setAttribute(
|
||||||
|
'aria-expanded',
|
||||||
|
String(queuePanel.hasAttribute('open')),
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
new MutationObserver(reflectQueueState).observe(queuePanel, {
|
||||||
|
attributes: true,
|
||||||
|
attributeFilter: ['open'],
|
||||||
|
});
|
||||||
|
|
||||||
|
reflectQueueState();
|
||||||
|
|
||||||
// ---------------------------------------------------------------
|
// ---------------------------------------------------------------
|
||||||
// Queue button as drop target (when queue panel is closed)
|
// Queue button as drop target (when queue panel is closed)
|
||||||
// ---------------------------------------------------------------
|
// ---------------------------------------------------------------
|
||||||
|
|||||||
@@ -68,6 +68,29 @@ export class SeekBar extends LitElement {
|
|||||||
align-items: center;
|
align-items: center;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* The clocks must not resize as they count.
|
||||||
|
|
||||||
|
Two things move them, and they need different answers. Digits in
|
||||||
|
a proportional font are different widths, so 1:11 is narrower
|
||||||
|
than 4:08 and the bar breathed once a second -- that is what
|
||||||
|
tabular figures fix. The character *count* changes too, at the
|
||||||
|
hundredth minute and whenever the right-hand clock is toggled to
|
||||||
|
remaining and grows a minus sign, and a figure width cannot fix
|
||||||
|
that -- so each clock also reserves the widest string this track
|
||||||
|
can put in it. The budget is per track rather than a constant
|
||||||
|
because reserving six characters on every track would push the
|
||||||
|
slider in by a character at each end for nothing. */
|
||||||
|
#seek-bar-container small,
|
||||||
|
.time-toggle {
|
||||||
|
font-variant-numeric: tabular-nums;
|
||||||
|
flex: 0 0 auto;
|
||||||
|
min-width: calc(var(--yj-clock-chars, 5) * 1ch);
|
||||||
|
}
|
||||||
|
|
||||||
|
#seek-bar-container small {
|
||||||
|
text-align: left;
|
||||||
|
}
|
||||||
|
|
||||||
.time-toggle {
|
.time-toggle {
|
||||||
background: none;
|
background: none;
|
||||||
border: none;
|
border: none;
|
||||||
@@ -76,6 +99,9 @@ export class SeekBar extends LitElement {
|
|||||||
font: inherit;
|
font: inherit;
|
||||||
font-size: var(--wa-font-size-s, 0.875rem);
|
font-size: var(--wa-font-size-s, 0.875rem);
|
||||||
cursor: pointer;
|
cursor: pointer;
|
||||||
|
/* One more for the minus sign the remaining form carries. */
|
||||||
|
min-width: calc((var(--yj-clock-chars, 5) + 1) * 1ch);
|
||||||
|
text-align: right;
|
||||||
}
|
}
|
||||||
|
|
||||||
.time-toggle:hover,
|
.time-toggle:hover,
|
||||||
@@ -219,8 +245,19 @@ export class SeekBar extends LitElement {
|
|||||||
: formatSeconds(this.trackLength);
|
: formatSeconds(this.trackLength);
|
||||||
const rightTime = this.hasTrack ? rightLabel : '--:--';
|
const rightTime = this.hasTrack ? rightLabel : '--:--';
|
||||||
|
|
||||||
|
// The widest string either clock can hold for *this* track. The
|
||||||
|
// duration is the longest elapsed value there can be, so its length
|
||||||
|
// is the budget; `--:--` is five, which is also the floor.
|
||||||
|
const clockChars = Math.max(
|
||||||
|
5,
|
||||||
|
this.hasTrack ? formatSeconds(this.trackLength).length : 0,
|
||||||
|
);
|
||||||
|
|
||||||
return html`
|
return html`
|
||||||
<div id="seek-bar-container">
|
<div
|
||||||
|
id="seek-bar-container"
|
||||||
|
style="--yj-clock-chars: ${clockChars}"
|
||||||
|
>
|
||||||
<small data-testid="elapsed-time">${elapsedTime}</small>
|
<small data-testid="elapsed-time">${elapsedTime}</small>
|
||||||
<wa-slider
|
<wa-slider
|
||||||
label="Seek"
|
label="Seek"
|
||||||
|
|||||||
@@ -111,13 +111,32 @@ const gridStyles = css`
|
|||||||
scale: 0.95;
|
scale: 0.95;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Title and year on one line, and only the title truncates.
|
||||||
|
|
||||||
|
The year used to be part of the same run of text, so it was the
|
||||||
|
first thing an ellipsis ate: a card wide enough for a long album
|
||||||
|
name never showed its year, and browsing by year showed years
|
||||||
|
only for the albums with short names -- the sort said one thing
|
||||||
|
and the cards showed another.
|
||||||
|
|
||||||
|
A flex row rather than a second line, because the card's height
|
||||||
|
is what the virtualizer measures rows by. */
|
||||||
.album-name {
|
.album-name {
|
||||||
font-size: var(--album-name-font, 14px);
|
font-size: var(--album-name-font, 14px);
|
||||||
font-weight: 400;
|
font-weight: 400;
|
||||||
color: var(--yj-text-primary, #fff);
|
color: var(--yj-text-primary, #fff);
|
||||||
|
display: flex;
|
||||||
|
justify-content: center;
|
||||||
|
align-items: baseline;
|
||||||
|
gap: 0.35em;
|
||||||
|
min-width: 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
.album-title {
|
||||||
white-space: nowrap;
|
white-space: nowrap;
|
||||||
overflow: hidden;
|
overflow: hidden;
|
||||||
text-overflow: ellipsis;
|
text-overflow: ellipsis;
|
||||||
|
min-width: 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
.artist-name {
|
.artist-name {
|
||||||
@@ -131,6 +150,8 @@ const gridStyles = css`
|
|||||||
|
|
||||||
.album-year {
|
.album-year {
|
||||||
color: var(--yj-text-tertiary, #888);
|
color: var(--yj-text-tertiary, #888);
|
||||||
|
flex: 0 0 auto;
|
||||||
|
white-space: nowrap;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* ========================================
|
/* ========================================
|
||||||
|
|||||||
@@ -1480,12 +1480,16 @@ export class CoverGrid
|
|||||||
source: 'cover-grid',
|
source: 'cover-grid',
|
||||||
});
|
});
|
||||||
|
|
||||||
// Single album: show cover art thumbnail.
|
// Single album: show its cover, badged with how many tracks are
|
||||||
// Multiple albums: show track-count badge.
|
// on the way -- an album is 1 track or 30 and the thumbnail is
|
||||||
|
// the same picture either way, so the number the drop is about
|
||||||
|
// was the one thing this drag did not say.
|
||||||
|
// Multiple albums: show the track-count badge alone.
|
||||||
if (isSingleAlbum && hit.album.CoverArtPath) {
|
if (isSingleAlbum && hit.album.CoverArtPath) {
|
||||||
this.dragImageEl =
|
this.dragImageEl =
|
||||||
createAlbumArtDragImage(
|
createAlbumArtDragImage(
|
||||||
this.getCoverUrl(hit.album),
|
this.getCoverUrl(hit.album),
|
||||||
|
filePaths.length,
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
this.dragImageEl = createDragImage(
|
this.dragImageEl = createDragImage(
|
||||||
@@ -1898,10 +1902,10 @@ export class CoverGrid
|
|||||||
class="album-name"
|
class="album-name"
|
||||||
title="${album.Name}"
|
title="${album.Name}"
|
||||||
>
|
>
|
||||||
${album.Name}${album.Year
|
<span class="album-title">${album.Name}</span
|
||||||
? html`
|
>${album.Year
|
||||||
<span class="album-year">
|
? html`<span class="album-year"
|
||||||
(${album.Year})</span
|
>(${album.Year})</span
|
||||||
>`
|
>`
|
||||||
: nothing}
|
: nothing}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -512,7 +512,9 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) {
|
|||||||
${request.artist ? `${request.artist} — ` : ''}${request.title ||
|
${request.artist ? `${request.artist} — ` : ''}${request.title ||
|
||||||
request.mbid}
|
request.mbid}
|
||||||
</div>
|
</div>
|
||||||
<div class="detail">${requestDetail(request, this.nowMs)}</div>
|
<div class="detail">
|
||||||
|
${requestDetail(request, this.nowMs, this.canDownload)}
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<div class="actions">
|
<div class="actions">
|
||||||
${request.state === 'satisfied'
|
${request.state === 'satisfied'
|
||||||
@@ -706,10 +708,20 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) {
|
|||||||
* looked for rather than as an error, because that is what it is — the
|
* looked for rather than as an error, because that is what it is — the
|
||||||
* retry is already scheduled and there is nothing for the user to do.
|
* retry is already scheduled and there is nothing for the user to do.
|
||||||
*/
|
*/
|
||||||
function requestDetail(request: Request, nowMs: number): string {
|
function requestDetail(
|
||||||
|
request: Request,
|
||||||
|
nowMs: number,
|
||||||
|
canDownload: boolean,
|
||||||
|
): string {
|
||||||
if (request.state === 'satisfied') return 'In your library';
|
if (request.state === 'satisfied') return 'In your library';
|
||||||
if (request.state === 'paused') return 'Paused — not being looked for';
|
if (request.state === 'paused') return 'Paused — not being looked for';
|
||||||
|
|
||||||
|
// With no client there is no search and no retry clock — the
|
||||||
|
// backend stopped scheduling one — so a row must not imply either.
|
||||||
|
// "Queued" and "next check in 6 hours" are both promises nothing is
|
||||||
|
// in a position to keep.
|
||||||
|
if (!canDownload) return 'On your list — no download client to search with';
|
||||||
|
|
||||||
if (request.attempts === 0) return 'Queued — not searched for yet';
|
if (request.attempts === 0) return 'Queued — not searched for yet';
|
||||||
|
|
||||||
const tries = `Searched ${request.attempts} time${request.attempts === 1 ? '' : 's'}`;
|
const tries = `Searched ${request.attempts} time${request.attempts === 1 ? '' : 's'}`;
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import { LitElement, html, css, nothing } from 'lit';
|
|||||||
import { customElement, property, state, query } from 'lit/decorators.js';
|
import { customElement, property, state, query } from 'lit/decorators.js';
|
||||||
import { classMap } from 'lit/directives/class-map.js';
|
import { classMap } from 'lit/directives/class-map.js';
|
||||||
import { designTokens } from '../../styles/tokens.css';
|
import { designTokens } from '../../styles/tokens.css';
|
||||||
|
import { srOnly } from '../../styles/sr-only.css';
|
||||||
import {
|
import {
|
||||||
LookupReleaseGroup,
|
LookupReleaseGroup,
|
||||||
BrowseReleases,
|
BrowseReleases,
|
||||||
@@ -289,6 +290,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
designTokens,
|
designTokens,
|
||||||
exploreLinkStyles,
|
exploreLinkStyles,
|
||||||
contextMenuStyles,
|
contextMenuStyles,
|
||||||
|
srOnly,
|
||||||
css`
|
css`
|
||||||
:host {
|
:host {
|
||||||
display: flex;
|
display: flex;
|
||||||
@@ -662,34 +664,21 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
font-weight: 400;
|
font-weight: 400;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* The request control is only offered where there is
|
/* The request control is offered on every row that has
|
||||||
* something to request, and only when the row is being
|
* something to request, and is not revealed on hover.
|
||||||
* attended to — a column of plus signs down a mostly-owned
|
|
||||||
* album is the clutter the green ticks were.
|
|
||||||
*
|
*
|
||||||
* Hidden with opacity, never display:none or visibility,
|
* It used to be transparent until the row was hovered or
|
||||||
* so it keeps its place in the layout (rows do not reflow
|
* focused, on the reasoning that a column of plus signs
|
||||||
* as the pointer moves) and stays in the tab order and the
|
* down a mostly-owned album is clutter. That reasoning was
|
||||||
* accessibility tree. focus-within is what makes it
|
* inherited from the green ticks it replaced and does not
|
||||||
* reachable without a mouse: tabbing to the button reveals
|
* survive the rule those were removed for: a tick marked
|
||||||
* it, and the row's own focus reveals it before you get
|
* the *common* case, while this marks the rows that are
|
||||||
* there. */
|
* **not** here. A mark on the exception is the information
|
||||||
|
* on this page — and one that appears only under the
|
||||||
|
* pointer cannot be seen, counted, or reached by anyone
|
||||||
|
* driving this with a finger. */
|
||||||
.track-row .track-request {
|
.track-row .track-request {
|
||||||
flex-shrink: 0;
|
flex-shrink: 0;
|
||||||
opacity: 0;
|
|
||||||
transition: opacity 0.12s ease;
|
|
||||||
}
|
|
||||||
|
|
||||||
.track-row:hover .track-request,
|
|
||||||
.track-row:focus-within .track-request,
|
|
||||||
.track-row .track-request:focus-visible {
|
|
||||||
opacity: 1;
|
|
||||||
}
|
|
||||||
|
|
||||||
@media (prefers-reduced-motion: reduce) {
|
|
||||||
.track-row .track-request {
|
|
||||||
transition: none;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
`,
|
`,
|
||||||
];
|
];
|
||||||
@@ -720,14 +709,28 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
|
|
||||||
// The download button only appears once a client is connected,
|
// The download button only appears once a client is connected,
|
||||||
// so this tracks the provider list rather than assuming.
|
// so this tracks the provider list rather than assuming.
|
||||||
|
//
|
||||||
|
// The `requestUpdate` is what makes the *tracklist's* badges
|
||||||
|
// move. Both assignments below are reactive fields, so Lit
|
||||||
|
// repaints when either changes — but a track request changes
|
||||||
|
// neither: `canDownload` is about providers and `isRequested`
|
||||||
|
// is about this album's own release group. Each row's badge
|
||||||
|
// reads `libraryStatusFor(false, track.mbid)` at render time,
|
||||||
|
// which is a dependency on the store that Lit cannot see, so
|
||||||
|
// clicking one filed the request and left the plus exactly
|
||||||
|
// where it was. The other three hosts rendering these badges
|
||||||
|
// (`explore-artist-details`, `explore-view`, `top-results-row`)
|
||||||
|
// have always asked for the repaint here; this one did not.
|
||||||
this.downloadUnsub = downloadStore.subscribe(() => {
|
this.downloadUnsub = downloadStore.subscribe(() => {
|
||||||
this.canDownload = downloadStore.available;
|
this.canDownload = downloadStore.available;
|
||||||
this.syncRequested();
|
this.syncRequested();
|
||||||
|
this.requestUpdate();
|
||||||
});
|
});
|
||||||
|
|
||||||
void downloadStore.init().then(() => {
|
void downloadStore.init().then(() => {
|
||||||
this.canDownload = downloadStore.available;
|
this.canDownload = downloadStore.available;
|
||||||
this.syncRequested();
|
this.syncRequested();
|
||||||
|
this.requestUpdate();
|
||||||
});
|
});
|
||||||
|
|
||||||
void this.resolveTargetLibraryId();
|
void this.resolveTargetLibraryId();
|
||||||
@@ -3033,11 +3036,21 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
|
|
||||||
/* ── Tracklist ── */
|
/* ── Tracklist ── */
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The heading is there and is not drawn.
|
||||||
|
*
|
||||||
|
* A list of numbered titles with durations under an album's cover
|
||||||
|
* does not need a word above it saying what it is — it was the
|
||||||
|
* only thing on this page labelling something already obvious. But
|
||||||
|
* the section is a landmark and the page's heading structure runs
|
||||||
|
* through it, so what goes is the *ink*, not the element: a reader
|
||||||
|
* jumping by heading still finds the tracklist.
|
||||||
|
*/
|
||||||
private renderTracklist() {
|
private renderTracklist() {
|
||||||
if (this.loadingReleases) {
|
if (this.loadingReleases) {
|
||||||
return html`
|
return html`
|
||||||
<section>
|
<section>
|
||||||
<h3 class="section-header">Tracklist</h3>
|
<h3 class="sr-only">Tracklist</h3>
|
||||||
<div class="section-loading">Loading tracks\u2026</div>
|
<div class="section-loading">Loading tracks\u2026</div>
|
||||||
</section>
|
</section>
|
||||||
`;
|
`;
|
||||||
@@ -3050,7 +3063,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
if (!current) {
|
if (!current) {
|
||||||
return html`
|
return html`
|
||||||
<section>
|
<section>
|
||||||
<h3 class="section-header">Tracklist</h3>
|
<h3 class="sr-only">Tracklist</h3>
|
||||||
<div class="section-error">
|
<div class="section-error">
|
||||||
<wa-icon name="triangle-exclamation"></wa-icon>
|
<wa-icon name="triangle-exclamation"></wa-icon>
|
||||||
No release data available.
|
No release data available.
|
||||||
@@ -3063,7 +3076,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
if (tracks.length === 0) {
|
if (tracks.length === 0) {
|
||||||
return html`
|
return html`
|
||||||
<section>
|
<section>
|
||||||
<h3 class="section-header">Tracklist</h3>
|
<h3 class="sr-only">Tracklist</h3>
|
||||||
<div
|
<div
|
||||||
style="color: var(--yj-text-tertiary, #888); font-size: var(--yj-text-md)"
|
style="color: var(--yj-text-tertiary, #888); font-size: var(--yj-text-md)"
|
||||||
>
|
>
|
||||||
@@ -3079,7 +3092,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
|
|
||||||
return html`
|
return html`
|
||||||
<section>
|
<section>
|
||||||
<h3 class="section-header">Tracklist</h3>
|
<h3 class="sr-only">Tracklist</h3>
|
||||||
<div class="tracklist">
|
<div class="tracklist">
|
||||||
${discNumbers.map((discNum) => {
|
${discNumbers.map((discNum) => {
|
||||||
const discTracks = discMap.get(discNum) ?? [];
|
const discTracks = discMap.get(discNum) ?? [];
|
||||||
|
|||||||
@@ -193,6 +193,10 @@ export class SmartPlaylistEditor extends LitElement {
|
|||||||
// ── Internal state ──────────────────────────────────────────────
|
// ── Internal state ──────────────────────────────────────────────
|
||||||
|
|
||||||
@state() private ruleRows: RuleRow[] = [emptyRule()];
|
@state() private ruleRows: RuleRow[] = [emptyRule()];
|
||||||
|
/** Whether every rule must hold or any one of them. Mirrors the
|
||||||
|
* backend's `match`; 'all' is the default and the only thing a
|
||||||
|
* playlist saved before this existed can have meant. */
|
||||||
|
@state() private matchType: 'all' | 'any' = 'all';
|
||||||
@state() private limit = 0;
|
@state() private limit = 0;
|
||||||
@state() private sortField = 'random';
|
@state() private sortField = 'random';
|
||||||
@state() private sortDir = '';
|
@state() private sortDir = '';
|
||||||
@@ -224,6 +228,19 @@ export class SmartPlaylistEditor extends LitElement {
|
|||||||
flex-shrink: 0;
|
flex-shrink: 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.match-row {
|
||||||
|
display: flex;
|
||||||
|
align-items: center;
|
||||||
|
gap: 6px;
|
||||||
|
font-size: var(--yj-text-sm);
|
||||||
|
color: var(--yj-text-secondary, #b3b3b3);
|
||||||
|
flex-wrap: wrap;
|
||||||
|
}
|
||||||
|
|
||||||
|
.match-select {
|
||||||
|
min-width: 72px;
|
||||||
|
}
|
||||||
|
|
||||||
.rule-row {
|
.rule-row {
|
||||||
display: grid;
|
display: grid;
|
||||||
grid-template-columns: 160px 140px 1fr 28px;
|
grid-template-columns: 160px 140px 1fr 28px;
|
||||||
@@ -511,6 +528,10 @@ export class SmartPlaylistEditor extends LitElement {
|
|||||||
);
|
);
|
||||||
|
|
||||||
this.ruleRows = rows.length > 0 ? rows : [emptyRule()];
|
this.ruleRows = rows.length > 0 ? rows : [emptyRule()];
|
||||||
|
// A playlist saved before this field existed has no match
|
||||||
|
// and means "all" — the backend reads an empty match the
|
||||||
|
// same way, so an upgrade cannot widen anyone's playlist.
|
||||||
|
this.matchType = parsed.match === 'any' ? 'any' : 'all';
|
||||||
this.limit = parsed.limit ?? 0;
|
this.limit = parsed.limit ?? 0;
|
||||||
this.sortField = parsed.sort_field || 'random';
|
this.sortField = parsed.sort_field || 'random';
|
||||||
this.sortDir = parsed.sort_dir ?? '';
|
this.sortDir = parsed.sort_dir ?? '';
|
||||||
@@ -551,6 +572,7 @@ export class SmartPlaylistEditor extends LitElement {
|
|||||||
|
|
||||||
return JSON.stringify({
|
return JSON.stringify({
|
||||||
rules,
|
rules,
|
||||||
|
match: this.matchType,
|
||||||
limit: this.limit || 0,
|
limit: this.limit || 0,
|
||||||
sort_field: this.sortField || '',
|
sort_field: this.sortField || '',
|
||||||
sort_dir: this.sortDir || '',
|
sort_dir: this.sortDir || '',
|
||||||
@@ -630,6 +652,11 @@ export class SmartPlaylistEditor extends LitElement {
|
|||||||
this.onRulesChanged();
|
this.onRulesChanged();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private updateMatchType(value: string) {
|
||||||
|
this.matchType = value === 'any' ? 'any' : 'all';
|
||||||
|
this.onRulesChanged();
|
||||||
|
}
|
||||||
|
|
||||||
private updateSortField(value: string) {
|
private updateSortField(value: string) {
|
||||||
this.sortField = value;
|
this.sortField = value;
|
||||||
if (!value) this.sortDir = '';
|
if (!value) this.sortDir = '';
|
||||||
@@ -731,6 +758,7 @@ export class SmartPlaylistEditor extends LitElement {
|
|||||||
override render() {
|
override render() {
|
||||||
return html`
|
return html`
|
||||||
<div class="rule-rows">
|
<div class="rule-rows">
|
||||||
|
${this.renderMatchType()}
|
||||||
${this.ruleRows.map((row, index) =>
|
${this.ruleRows.map((row, index) =>
|
||||||
this.renderRuleRow(row, index),
|
this.renderRuleRow(row, index),
|
||||||
)}
|
)}
|
||||||
@@ -743,6 +771,46 @@ export class SmartPlaylistEditor extends LitElement {
|
|||||||
`;
|
`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether every rule has to hold, or any one of them.
|
||||||
|
*
|
||||||
|
* It is a sentence with a control in the middle rather than a
|
||||||
|
* labelled field, because the two readings differ by one word and
|
||||||
|
* that word is the whole of the setting — "Match **all** of the
|
||||||
|
* following rules" says what the list below it means in a way a
|
||||||
|
* select labelled "Match" beside a list does not.
|
||||||
|
*
|
||||||
|
* Hidden while there is one rule: with nothing to combine, all and
|
||||||
|
* any are the same query, and a control whose two settings cannot
|
||||||
|
* differ is a question the user has no way to answer wrongly and
|
||||||
|
* no reason to answer at all.
|
||||||
|
*/
|
||||||
|
private renderMatchType() {
|
||||||
|
if (this.ruleRows.length < 2) return nothing;
|
||||||
|
|
||||||
|
return html`
|
||||||
|
<div class="match-row">
|
||||||
|
<span>Match</span>
|
||||||
|
<select
|
||||||
|
class="match-select"
|
||||||
|
aria-label="Match all or any of the following rules"
|
||||||
|
@change=${(e: Event) =>
|
||||||
|
this.updateMatchType(
|
||||||
|
(e.target as HTMLSelectElement).value,
|
||||||
|
)}
|
||||||
|
>
|
||||||
|
<option value="all" ?selected=${this.matchType === 'all'}>
|
||||||
|
all
|
||||||
|
</option>
|
||||||
|
<option value="any" ?selected=${this.matchType === 'any'}>
|
||||||
|
any
|
||||||
|
</option>
|
||||||
|
</select>
|
||||||
|
<span>of the following rules</span>
|
||||||
|
</div>
|
||||||
|
`;
|
||||||
|
}
|
||||||
|
|
||||||
private renderRuleRow(row: RuleRow, index: number) {
|
private renderRuleRow(row: RuleRow, index: number) {
|
||||||
const isBetween = row.operator === 'between';
|
const isBetween = row.operator === 'between';
|
||||||
const operators = row.field ? getOperatorsForField(row.field) : [];
|
const operators = row.field ? getOperatorsForField(row.field) : [];
|
||||||
|
|||||||
@@ -57,6 +57,12 @@ interface TracksModified {
|
|||||||
index: number;
|
index: number;
|
||||||
positions?: number[];
|
positions?: number[];
|
||||||
currentIndex: number;
|
currentIndex: number;
|
||||||
|
/** The queue's source *after* the mutation. An append clears it
|
||||||
|
* backend-side — a queue built from one album is not that album
|
||||||
|
* once a track from elsewhere joins it — and this delta is the
|
||||||
|
* only event those paths emit, so the label would otherwise keep
|
||||||
|
* pointing at a collection the queue no longer holds. */
|
||||||
|
source?: QueueSource;
|
||||||
}
|
}
|
||||||
|
|
||||||
type Subscriber = () => void;
|
type Subscriber = () => void;
|
||||||
@@ -198,6 +204,7 @@ class QueueStore {
|
|||||||
}
|
}
|
||||||
|
|
||||||
this.state.currentIndex = delta.currentIndex;
|
this.state.currentIndex = delta.currentIndex;
|
||||||
|
this.state.source = delta.source ?? EMPTY_QUEUE_SOURCE;
|
||||||
}
|
}
|
||||||
|
|
||||||
// ===================================================================
|
// ===================================================================
|
||||||
|
|||||||
@@ -29,11 +29,23 @@ export function createDragImage(count: number): HTMLElement {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Creates a drag image showing an album cover art thumbnail.
|
* Creates a drag image showing an album cover art thumbnail, with a
|
||||||
* Falls back to the track-count badge if the image fails to load.
|
* corner badge saying how many tracks are on the way.
|
||||||
|
*
|
||||||
|
* The count is not decoration. The cover says *what* is being dragged
|
||||||
|
* and nothing said *how much* — an album is 1 track or 30 and the
|
||||||
|
* thumbnail is identical either way, so the one number the drop is
|
||||||
|
* about was the one thing the drag did not show. Every other drag in
|
||||||
|
* the app says it (`createDragImage` is a count and nothing else);
|
||||||
|
* this one was the exception because it had a picture to show instead.
|
||||||
|
*
|
||||||
|
* A count of 1 draws no badge: "1" over a single album cover is noise,
|
||||||
|
* and the absence is unambiguous next to a badge that only ever
|
||||||
|
* appears when there is more than one.
|
||||||
*/
|
*/
|
||||||
export function createAlbumArtDragImage(
|
export function createAlbumArtDragImage(
|
||||||
coverUrl: string,
|
coverUrl: string,
|
||||||
|
count = 1,
|
||||||
): HTMLElement {
|
): HTMLElement {
|
||||||
const size = 64;
|
const size = 64;
|
||||||
const wrapper = document.createElement('div');
|
const wrapper = document.createElement('div');
|
||||||
@@ -44,6 +56,12 @@ export function createAlbumArtDragImage(
|
|||||||
'left: -1000px',
|
'left: -1000px',
|
||||||
'pointer-events: none',
|
'pointer-events: none',
|
||||||
'z-index: 9999',
|
'z-index: 9999',
|
||||||
|
// The badge is positioned against this box, and the box stays
|
||||||
|
// exactly the cover's size: anything outside it risks being
|
||||||
|
// clipped out of the snapshot the browser takes, and padding
|
||||||
|
// it instead would move the cover away from the cursor.
|
||||||
|
`width: ${size}px`,
|
||||||
|
`height: ${size}px`,
|
||||||
].join(';');
|
].join(';');
|
||||||
|
|
||||||
const img = document.createElement('img');
|
const img = document.createElement('img');
|
||||||
@@ -61,11 +79,44 @@ export function createAlbumArtDragImage(
|
|||||||
].join(';');
|
].join(';');
|
||||||
|
|
||||||
wrapper.appendChild(img);
|
wrapper.appendChild(img);
|
||||||
|
|
||||||
|
if (count > 1) {
|
||||||
|
wrapper.appendChild(countBadge(count));
|
||||||
|
}
|
||||||
|
|
||||||
document.body.appendChild(wrapper);
|
document.body.appendChild(wrapper);
|
||||||
|
|
||||||
return wrapper;
|
return wrapper;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** The corner badge on a multi-track drag image. */
|
||||||
|
function countBadge(count: number): HTMLElement {
|
||||||
|
const badge = document.createElement('span');
|
||||||
|
|
||||||
|
badge.className = 'drag-count-badge';
|
||||||
|
badge.textContent = String(count);
|
||||||
|
badge.style.cssText = [
|
||||||
|
'position: absolute',
|
||||||
|
'top: 3px',
|
||||||
|
'right: 3px',
|
||||||
|
'min-width: 20px',
|
||||||
|
'height: 20px',
|
||||||
|
'padding: 0 5px',
|
||||||
|
'box-sizing: border-box',
|
||||||
|
'border-radius: 10px',
|
||||||
|
'background: #ffd43b',
|
||||||
|
'color: #000',
|
||||||
|
'font-size: 12px',
|
||||||
|
'font-weight: 600',
|
||||||
|
'font-family: inherit',
|
||||||
|
'line-height: 20px',
|
||||||
|
'text-align: center',
|
||||||
|
'box-shadow: 0 1px 4px rgba(0,0,0,0.5)',
|
||||||
|
].join(';');
|
||||||
|
|
||||||
|
return badge;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Creates a drag image styled like a queue track card showing the
|
* Creates a drag image styled like a queue track card showing the
|
||||||
* track title and artist. Used when dragging a single track.
|
* track title and artist. Used when dragging a single track.
|
||||||
|
|||||||
@@ -0,0 +1,87 @@
|
|||||||
|
/**
|
||||||
|
* The year on an album card survives a long album name.
|
||||||
|
*
|
||||||
|
* The year used to be part of the same run of text as the title, inside
|
||||||
|
* one `text-overflow: ellipsis` box — so it was the first thing the
|
||||||
|
* ellipsis ate. A card wide enough for a long name never showed its
|
||||||
|
* year at all, which means sorting the grid *by year* showed years only
|
||||||
|
* for the albums with short names: the sort said one thing and the
|
||||||
|
* cards showed another.
|
||||||
|
*
|
||||||
|
* The fix is a flex row in which only the title truncates, rather than
|
||||||
|
* a second line, because the card's height is what the virtualizer
|
||||||
|
* measures rows by.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it, beforeEach } from 'vitest';
|
||||||
|
import type { LitElement } from 'lit';
|
||||||
|
|
||||||
|
import '@components/cover-grid/cover-grid';
|
||||||
|
import { emit, stub, flush, resetHarness } from '@test/support/harness';
|
||||||
|
import { Events } from '../../src/events';
|
||||||
|
import { fixture, shadowAll } from '@test/support/render';
|
||||||
|
|
||||||
|
const LONG =
|
||||||
|
'The Rise and Fall of a Midwest Princess in the Key of Everything';
|
||||||
|
|
||||||
|
/** Long names throughout: the fault only shows on a card under
|
||||||
|
* pressure, and a grid of "Album 3" proves nothing. */
|
||||||
|
const ALBUMS = Array.from({ length: 12 }, (_, i) => ({
|
||||||
|
ID: i + 1,
|
||||||
|
Name: `${LONG} ${i + 1}`,
|
||||||
|
ArtistName: 'Aurora Fields',
|
||||||
|
Year: 2019 + (i % 5),
|
||||||
|
}));
|
||||||
|
|
||||||
|
/** Give the virtualizer a viewport; a zero-height host renders nothing. */
|
||||||
|
function sized(el: HTMLElement): void {
|
||||||
|
el.style.display = 'block';
|
||||||
|
el.style.height = '600px';
|
||||||
|
el.style.width = '900px';
|
||||||
|
}
|
||||||
|
|
||||||
|
async function settle(el: LitElement): Promise<void> {
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
await new Promise((r) => setTimeout(r, 80));
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('the album card’s year', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetHarness();
|
||||||
|
stub('library.Library.GetAlbums', ALBUMS);
|
||||||
|
stub('library.Library.GetTracks', []);
|
||||||
|
emit(Events.LibraryScanComplete);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is rendered on every card, however long the name', async () => {
|
||||||
|
const el = await fixture<LitElement>('cover-grid');
|
||||||
|
|
||||||
|
sized(el);
|
||||||
|
await settle(el);
|
||||||
|
|
||||||
|
const cards = shadowAll(el, '.album-card');
|
||||||
|
const years = shadowAll(el, '.album-year');
|
||||||
|
|
||||||
|
expect(cards.length).toBeGreaterThan(0);
|
||||||
|
expect(years).toHaveLength(cards.length);
|
||||||
|
expect(years.every((y) => /^\(\d{4}\)$/.test(y.textContent!.trim()))).toBe(
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is not what the ellipsis eats', async () => {
|
||||||
|
const el = await fixture<LitElement>('cover-grid');
|
||||||
|
|
||||||
|
sized(el);
|
||||||
|
await settle(el);
|
||||||
|
|
||||||
|
const year = shadowAll(el, '.album-year')[0]!;
|
||||||
|
const title = shadowAll(el, '.album-title')[0]!;
|
||||||
|
|
||||||
|
// The title is the box that gives way...
|
||||||
|
expect(title.scrollWidth).toBeGreaterThan(title.clientWidth);
|
||||||
|
// ...and the year keeps every pixel it asked for.
|
||||||
|
expect(year.clientWidth).toBeGreaterThan(0);
|
||||||
|
expect(year.scrollWidth).toBeLessThanOrEqual(year.clientWidth + 1);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,97 @@
|
|||||||
|
/**
|
||||||
|
* The request badge on an unowned row is there without being hovered.
|
||||||
|
*
|
||||||
|
* It used to be transparent until the row was hovered or focused, on
|
||||||
|
* the reasoning that a column of plus signs down a mostly-owned album
|
||||||
|
* is clutter. That reasoning came from the green ticks it replaced and
|
||||||
|
* does not survive the rule those were removed for: a tick marked the
|
||||||
|
* **common** case, while this marks the rows that are *not* here. A
|
||||||
|
* mark on the exception is the information on this page, and one that
|
||||||
|
* exists only under the pointer cannot be seen, counted, or reached by
|
||||||
|
* anyone driving the app with a finger.
|
||||||
|
*
|
||||||
|
* That the badge *repaints* when clicked is the other half of #33 and
|
||||||
|
* is covered by `album-track-request.test.ts`.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it, beforeEach } from 'vitest';
|
||||||
|
import type { LitElement } from 'lit';
|
||||||
|
|
||||||
|
import '@components/explore-album-details/explore-album-details';
|
||||||
|
import { stub, flush, resetHarness } from '@test/support/harness';
|
||||||
|
import { fixture, shadowAll } from '@test/support/render';
|
||||||
|
|
||||||
|
function track(n: number, owned: boolean) {
|
||||||
|
return {
|
||||||
|
position: n,
|
||||||
|
discNumber: 1,
|
||||||
|
title: `Track ${n}`,
|
||||||
|
length: 200000,
|
||||||
|
mbid: `mbid-${n}`,
|
||||||
|
inLibrary: owned,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/** An album with one owned track and one that is not here. */
|
||||||
|
async function albumWithAnUnownedTrack(): Promise<LitElement> {
|
||||||
|
const el = await fixture<LitElement>('explore-album-details', {
|
||||||
|
albumName: 'Glass Harbour',
|
||||||
|
releaseGroupMBID: 'rg-1',
|
||||||
|
});
|
||||||
|
|
||||||
|
stub('library.Library.GetFilePathsByRecordingMBIDs', {
|
||||||
|
'mbid-1': ['/music/mbid-1.mp3'],
|
||||||
|
});
|
||||||
|
|
||||||
|
Object.assign(el, {
|
||||||
|
versionEntries: [
|
||||||
|
{
|
||||||
|
key: 'v1',
|
||||||
|
label: '2019',
|
||||||
|
sublabel: '2 tracks',
|
||||||
|
tracks: [track(1, true), track(2, false)],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
selectedVersionKey: 'v1',
|
||||||
|
loadingReleases: false,
|
||||||
|
loadingInfo: false,
|
||||||
|
});
|
||||||
|
el.requestUpdate();
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
return el;
|
||||||
|
}
|
||||||
|
|
||||||
|
const badges = (el: LitElement) =>
|
||||||
|
shadowAll(el, 'library-status-indicator.track-request');
|
||||||
|
|
||||||
|
describe('the tracklist’s request badge', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetHarness();
|
||||||
|
stub('library.Library.GetFilePathsByRecordingMBIDs', {});
|
||||||
|
stub('library.Library.GetFilePathsByAlbums', {});
|
||||||
|
stub('library.Library.GetAlbumTracks', []);
|
||||||
|
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
|
||||||
|
stub('download.Service.ProviderKinds', []);
|
||||||
|
stub('download.Service.ListProviders', []);
|
||||||
|
stub('download.Service.ListDownloads', []);
|
||||||
|
stub('download.Service.ListRequests', []);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is visible without a pointer anywhere near it', async () => {
|
||||||
|
const el = await albumWithAnUnownedTrack();
|
||||||
|
const [badge] = badges(el);
|
||||||
|
|
||||||
|
expect(badge).toBeTruthy();
|
||||||
|
// Computed opacity rather than the absence of a rule, because the
|
||||||
|
// rule could come back under a different selector.
|
||||||
|
expect(getComputedStyle(badge!).opacity).toBe('1');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is still only on the rows with something to request', async () => {
|
||||||
|
// Always-visible is not the same as everywhere: an owned track has
|
||||||
|
// nothing left to ask for, and a badge on it would be the column of
|
||||||
|
// green ticks this page deliberately stopped drawing.
|
||||||
|
expect(badges(await albumWithAnUnownedTrack())).toHaveLength(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,149 @@
|
|||||||
|
/**
|
||||||
|
* Issue #33: "Want track" filed the request and left the badge alone.
|
||||||
|
*
|
||||||
|
* The tracklist's badges read `libraryStatusFor(false, track.mbid)` at
|
||||||
|
* render time, which is a dependency on `downloadStore` that Lit cannot
|
||||||
|
* see. `explore-album-details` did subscribe to that store, but its
|
||||||
|
* callback only assigned `canDownload` and `isRequested` — neither of
|
||||||
|
* which a *track* request changes — so nothing in the component's
|
||||||
|
* reactive state moved and the page never re-rendered. The request was
|
||||||
|
* real, the plus stayed a plus, and clicking again cancelled it.
|
||||||
|
*
|
||||||
|
* The other three hosts rendering these badges (`explore-artist-
|
||||||
|
* details`, `explore-view`, `top-results-row`) have always asked for
|
||||||
|
* the repaint in the same place, which is what made this one look
|
||||||
|
* correct on inspection.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it, beforeEach } from 'vitest';
|
||||||
|
import type { LitElement } from 'lit';
|
||||||
|
|
||||||
|
import '@components/explore-album-details/explore-album-details';
|
||||||
|
import type { Request } from '@store/download-store';
|
||||||
|
import { Events } from '../../src/events';
|
||||||
|
import { stub, flush, resetHarness, emit } from '@test/support/harness';
|
||||||
|
import { fixture, shadowAll } from '@test/support/render';
|
||||||
|
|
||||||
|
type RequestOverrides = Partial<Omit<Request, 'state' | 'entity'>> & {
|
||||||
|
state?: `${Request['state']}`;
|
||||||
|
entity?: `${Request['entity']}`;
|
||||||
|
};
|
||||||
|
|
||||||
|
function request(overrides: RequestOverrides): Request {
|
||||||
|
return {
|
||||||
|
id: 1,
|
||||||
|
mbid: 'mbid-1',
|
||||||
|
entity: 'recording',
|
||||||
|
libraryId: 1,
|
||||||
|
artist: 'An Artist',
|
||||||
|
title: 'Track 1',
|
||||||
|
scope: 'future',
|
||||||
|
secondary: false,
|
||||||
|
state: 'wanted',
|
||||||
|
attempts: 0,
|
||||||
|
...overrides,
|
||||||
|
} as Request;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Put a request list into the store the way the backend does. */
|
||||||
|
async function withRequests(rows: Request[]): Promise<void> {
|
||||||
|
stub('download.Service.ListRequests', rows);
|
||||||
|
emit(Events.RequestsChanged);
|
||||||
|
await flush();
|
||||||
|
}
|
||||||
|
|
||||||
|
function track(n: number) {
|
||||||
|
return {
|
||||||
|
position: n,
|
||||||
|
discNumber: 1,
|
||||||
|
title: `Track ${n}`,
|
||||||
|
length: 200000,
|
||||||
|
mbid: `mbid-${n}`,
|
||||||
|
inLibrary: false,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/** An unowned catalog tracklist, which is the only case with badges:
|
||||||
|
* an owned row renders none, there being nothing left to ask for. */
|
||||||
|
async function withTracklist(count: number): Promise<LitElement> {
|
||||||
|
const el = await fixture<LitElement>('explore-album-details', {
|
||||||
|
albumName: 'Glass Harbour',
|
||||||
|
});
|
||||||
|
|
||||||
|
Object.assign(el, {
|
||||||
|
versionEntries: [
|
||||||
|
{
|
||||||
|
key: 'v1',
|
||||||
|
label: '2019',
|
||||||
|
sublabel: `${count} tracks`,
|
||||||
|
tracks: Array.from({ length: count }, (_, i) => track(i + 1)),
|
||||||
|
},
|
||||||
|
],
|
||||||
|
selectedVersionKey: 'v1',
|
||||||
|
loadingReleases: false,
|
||||||
|
loadingInfo: false,
|
||||||
|
});
|
||||||
|
el.requestUpdate();
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
return el;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** The status of each track badge, in tracklist order. */
|
||||||
|
function badgeStatuses(el: LitElement): string[] {
|
||||||
|
return shadowAll(el, 'library-status-indicator.track-request').map(
|
||||||
|
(b) => b.getAttribute('status') ?? '',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('the album tracklist’s request badges', () => {
|
||||||
|
beforeEach(async () => {
|
||||||
|
resetHarness();
|
||||||
|
stub('library.Library.GetFilePathsByRecordingMBIDs', {});
|
||||||
|
stub('library.Library.GetFilePathsByAlbums', {});
|
||||||
|
stub('library.Library.GetAlbumTracks', []);
|
||||||
|
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
|
||||||
|
await withRequests([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('starts as a plus on every unowned row', async () => {
|
||||||
|
const el = await withTracklist(3);
|
||||||
|
|
||||||
|
expect(badgeStatuses(el)).toEqual([
|
||||||
|
'not-in-library',
|
||||||
|
'not-in-library',
|
||||||
|
'not-in-library',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('repaints the row whose track has been requested', async () => {
|
||||||
|
const el = await withTracklist(3);
|
||||||
|
|
||||||
|
await withRequests([request({ mbid: 'mbid-2' })]);
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
// Only the requested row moves. A request is by MBID, so the two
|
||||||
|
// rows either side of it are still a plus.
|
||||||
|
expect(badgeStatuses(el)).toEqual([
|
||||||
|
'not-in-library',
|
||||||
|
'queued',
|
||||||
|
'not-in-library',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('repaints again when the request is cancelled', async () => {
|
||||||
|
const el = await withTracklist(3);
|
||||||
|
|
||||||
|
await withRequests([request({ mbid: 'mbid-2' })]);
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
await withRequests([]);
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
expect(badgeStatuses(el)).toEqual([
|
||||||
|
'not-in-library',
|
||||||
|
'not-in-library',
|
||||||
|
'not-in-library',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,89 @@
|
|||||||
|
/**
|
||||||
|
* The tracklist's own heading.
|
||||||
|
*
|
||||||
|
* A list of numbered titles with durations, under the album's cover, is
|
||||||
|
* the one thing on this page that did not need a word above it saying
|
||||||
|
* what it was — "TRACKLIST" labelled the only thing already obvious.
|
||||||
|
*
|
||||||
|
* What goes is the *ink*, not the element. The section is a landmark
|
||||||
|
* and the page's heading structure runs through it, so a reader moving
|
||||||
|
* by heading still has to be able to find it, and it is hidden the way
|
||||||
|
* `sr-only` hides things: `clip-path`, never `display: none`, which
|
||||||
|
* would take it out of the accessibility tree along with the layout.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it, beforeEach } from 'vitest';
|
||||||
|
import type { LitElement } from 'lit';
|
||||||
|
|
||||||
|
import '@components/explore-album-details/explore-album-details';
|
||||||
|
import { stub, flush, resetHarness } from '@test/support/harness';
|
||||||
|
import { fixture, shadowAll } from '@test/support/render';
|
||||||
|
|
||||||
|
function track(n: number) {
|
||||||
|
return {
|
||||||
|
position: n,
|
||||||
|
discNumber: 1,
|
||||||
|
title: `Track ${n}`,
|
||||||
|
length: 200000,
|
||||||
|
mbid: `mbid-${n}`,
|
||||||
|
inLibrary: true,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
async function albumPage(): Promise<LitElement> {
|
||||||
|
const el = await fixture<LitElement>('explore-album-details', {
|
||||||
|
albumName: 'Glass Harbour',
|
||||||
|
releaseGroupMBID: 'rg-1',
|
||||||
|
});
|
||||||
|
|
||||||
|
Object.assign(el, {
|
||||||
|
versionEntries: [
|
||||||
|
{
|
||||||
|
key: 'v1',
|
||||||
|
label: '2019',
|
||||||
|
sublabel: '2 tracks',
|
||||||
|
tracks: [track(1), track(2)],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
selectedVersionKey: 'v1',
|
||||||
|
loadingReleases: false,
|
||||||
|
loadingInfo: false,
|
||||||
|
});
|
||||||
|
el.requestUpdate();
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
return el;
|
||||||
|
}
|
||||||
|
|
||||||
|
const tracklistHeading = (el: LitElement) =>
|
||||||
|
shadowAll(el, 'h3').find((h) => h.textContent?.trim() === 'Tracklist');
|
||||||
|
|
||||||
|
describe('the album tracklist heading', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetHarness();
|
||||||
|
stub('library.Library.GetFilePathsByRecordingMBIDs', {});
|
||||||
|
stub('library.Library.GetFilePathsByAlbums', {});
|
||||||
|
stub('library.Library.GetAlbumTracks', []);
|
||||||
|
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
|
||||||
|
stub('download.Service.ProviderKinds', []);
|
||||||
|
stub('download.Service.ListProviders', []);
|
||||||
|
stub('download.Service.ListDownloads', []);
|
||||||
|
stub('download.Service.ListRequests', []);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is still in the tree', async () => {
|
||||||
|
expect(tracklistHeading(await albumPage())).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('takes up no room on the page', async () => {
|
||||||
|
const heading = tracklistHeading(await albumPage())!;
|
||||||
|
const box = heading.getBoundingClientRect();
|
||||||
|
|
||||||
|
expect(box.width).toBeLessThanOrEqual(1);
|
||||||
|
expect(box.height).toBeLessThanOrEqual(1);
|
||||||
|
// Hidden by clipping, not by removal: display:none and
|
||||||
|
// visibility:hidden both take it out of the accessibility tree.
|
||||||
|
expect(getComputedStyle(heading).display).not.toBe('none');
|
||||||
|
expect(getComputedStyle(heading).visibility).not.toBe('hidden');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,90 @@
|
|||||||
|
/**
|
||||||
|
* What a wanted list says when there is nothing to search with.
|
||||||
|
*
|
||||||
|
* Wanting something without a download client is a supported thing to
|
||||||
|
* do — the list is kept, and it starts moving when a client is added.
|
||||||
|
* What was not supported was the app *claiming to be looking*: every
|
||||||
|
* pass attempted each request, failed it with "no download clients are
|
||||||
|
* enabled", recorded that as an attempt and scheduled a retry, so a row
|
||||||
|
* read "Searched 3 times, no download clients are enabled · next check
|
||||||
|
* in 6 hours" about a check that could not happen.
|
||||||
|
*
|
||||||
|
* The backend half is `TestNoProvidersMeansNoAttempt`. This is the row.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it, beforeEach } from 'vitest';
|
||||||
|
import type { LitElement } from 'lit';
|
||||||
|
|
||||||
|
import '@components/downloads-view/downloads-view';
|
||||||
|
import { stub, emit, flush, resetHarness } from '@test/support/harness';
|
||||||
|
import { Events } from '../../src/events';
|
||||||
|
import { fixture, shadowAll } from '@test/support/render';
|
||||||
|
|
||||||
|
/** A request that has been tried and is waiting on a retry — the shape
|
||||||
|
* a list with a client in it produces. */
|
||||||
|
const WAITING = {
|
||||||
|
id: 1,
|
||||||
|
mbid: 'rg-1',
|
||||||
|
entity: 'release-group',
|
||||||
|
libraryId: 1,
|
||||||
|
artist: 'Aurora Fields',
|
||||||
|
title: 'Glass Harbour',
|
||||||
|
state: 'wanted',
|
||||||
|
attempts: 3,
|
||||||
|
lastError: 'no source has it yet',
|
||||||
|
nextTryAt: new Date(Date.now() + 6 * 3600_000).toISOString(),
|
||||||
|
};
|
||||||
|
|
||||||
|
const PROVIDER = {
|
||||||
|
id: 1,
|
||||||
|
kind: 'slskd',
|
||||||
|
name: 'Sound',
|
||||||
|
enabled: true,
|
||||||
|
priority: 50,
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The download store is a singleton whose `init()` runs once per
|
||||||
|
* session, so a second mount does not re-read the provider list. The
|
||||||
|
* event is how the app itself learns a client was added, and is what
|
||||||
|
* makes this test independent of which case ran first.
|
||||||
|
*/
|
||||||
|
async function view(providers: unknown[]): Promise<LitElement> {
|
||||||
|
stub('download.Service.ListProviders', providers);
|
||||||
|
|
||||||
|
const el = await fixture<LitElement>('downloads-view');
|
||||||
|
|
||||||
|
emit(Events.DownloadProvidersChanged);
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
return el;
|
||||||
|
}
|
||||||
|
|
||||||
|
const details = (el: LitElement) =>
|
||||||
|
shadowAll(el, '.detail').map((d) => d.textContent!.trim());
|
||||||
|
|
||||||
|
describe('a request row with no download client', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetHarness();
|
||||||
|
stub('download.Service.ProviderKinds', []);
|
||||||
|
stub('download.Service.ListProviders', []);
|
||||||
|
stub('download.Service.ListDownloads', []);
|
||||||
|
stub('download.Service.ListRequests', [WAITING]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not promise a check that cannot happen', async () => {
|
||||||
|
const el = await view([]);
|
||||||
|
|
||||||
|
expect(details(el)).toHaveLength(1);
|
||||||
|
expect(details(el)[0]).toBe(
|
||||||
|
'On your list — no download client to search with',
|
||||||
|
);
|
||||||
|
expect(details(el)[0]).not.toMatch(/next check/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('reports the retry schedule again once a client exists', async () => {
|
||||||
|
const el = await view([PROVIDER]);
|
||||||
|
|
||||||
|
expect(details(el)[0]).toMatch(/next check/);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,71 @@
|
|||||||
|
/**
|
||||||
|
* A drag says how much it is carrying.
|
||||||
|
*
|
||||||
|
* Every drag in the app already did — `createDragImage` is a count and
|
||||||
|
* nothing else — except the one with a picture to show instead. An
|
||||||
|
* album dragged to the queue put its cover under the cursor and said
|
||||||
|
* nothing about how many tracks that was, and an album is 1 track or 30
|
||||||
|
* with the same thumbnail either way. The number is the thing the drop
|
||||||
|
* is about.
|
||||||
|
*
|
||||||
|
* A count of 1 draws no badge: "1" over a single cover is noise, and
|
||||||
|
* the absence reads unambiguously beside a badge that only ever appears
|
||||||
|
* when there is more than one.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it, afterEach } from 'vitest';
|
||||||
|
|
||||||
|
import {
|
||||||
|
createAlbumArtDragImage,
|
||||||
|
removeDragImage,
|
||||||
|
} from '@utils/drag-image';
|
||||||
|
|
||||||
|
const made: HTMLElement[] = [];
|
||||||
|
|
||||||
|
function dragImage(count?: number): HTMLElement {
|
||||||
|
const el =
|
||||||
|
count === undefined
|
||||||
|
? createAlbumArtDragImage('data:image/gif;base64,R0lGODlhAQABAAAAACw=')
|
||||||
|
: createAlbumArtDragImage(
|
||||||
|
'data:image/gif;base64,R0lGODlhAQABAAAAACw=',
|
||||||
|
count,
|
||||||
|
);
|
||||||
|
|
||||||
|
made.push(el);
|
||||||
|
|
||||||
|
return el;
|
||||||
|
}
|
||||||
|
|
||||||
|
const badge = (el: HTMLElement) =>
|
||||||
|
el.querySelector<HTMLElement>('.drag-count-badge');
|
||||||
|
|
||||||
|
describe('the album drag image', () => {
|
||||||
|
afterEach(() => {
|
||||||
|
while (made.length > 0) removeDragImage(made.pop()!);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('says how many tracks are being dragged', () => {
|
||||||
|
expect(badge(dragImage(12))?.textContent).toBe('12');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('says nothing when there is only one track', () => {
|
||||||
|
expect(badge(dragImage(1))).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still draws a bare cover for a caller that gives no count', () => {
|
||||||
|
// The count is optional so the helper stays usable from a call site
|
||||||
|
// that has a cover and no list; it must not badge such a drag "1".
|
||||||
|
expect(badge(dragImage())).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps the badge inside the cover', () => {
|
||||||
|
// setDragImage snapshots the element, and anything outside its box
|
||||||
|
// risks being clipped out of that snapshot — while padding the box
|
||||||
|
// instead would move the cover away from the cursor.
|
||||||
|
const el = dragImage(30);
|
||||||
|
const outer = el.getBoundingClientRect();
|
||||||
|
const mark = badge(el)!.getBoundingClientRect();
|
||||||
|
|
||||||
|
expect(mark.right).toBeLessThanOrEqual(outer.right);
|
||||||
|
expect(mark.top).toBeGreaterThanOrEqual(outer.top);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -227,6 +227,41 @@ describe('<seek-bar>', () => {
|
|||||||
expect(text(el, '[data-testid="remaining-time"]')).toBe('01:30');
|
expect(text(el, '[data-testid="remaining-time"]')).toBe('01:30');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('keeps the slider still as the clocks count', async () => {
|
||||||
|
const el = await fixture('seek-bar');
|
||||||
|
|
||||||
|
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 31 });
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
const slider = () =>
|
||||||
|
shadow(el, 'wa-slider')!.getBoundingClientRect();
|
||||||
|
const before = slider();
|
||||||
|
|
||||||
|
// 1:11 against 4:08 is the reported jitter: different digits, and
|
||||||
|
// in a proportional font different widths. Toggling the right-hand
|
||||||
|
// clock is the other half -- the minus sign is a whole character.
|
||||||
|
for (const positionSeconds of [8, 71, 88]) {
|
||||||
|
emit(Events.PlaybackPositionChanged, {
|
||||||
|
positionSeconds,
|
||||||
|
trackLength: 90,
|
||||||
|
trackChangeId: 31,
|
||||||
|
seq: positionSeconds,
|
||||||
|
playing: true,
|
||||||
|
});
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
expect(slider().width).toBeCloseTo(before.width, 1);
|
||||||
|
expect(slider().left).toBeCloseTo(before.left, 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
await click(el, '[data-testid="remaining-time"]');
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
expect(slider().width).toBeCloseTo(before.width, 1);
|
||||||
|
});
|
||||||
|
|
||||||
it('renders the position the backend reports rather than its own count', async () => {
|
it('renders the position the backend reports rather than its own count', async () => {
|
||||||
const el = await fixture('seek-bar');
|
const el = await fixture('seek-bar');
|
||||||
|
|
||||||
|
|||||||
@@ -198,6 +198,61 @@ describe('queue store: move', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* "Playing from X" is a claim that everything queued came from X, and
|
||||||
|
* appending a track from anywhere else makes it false. The backend
|
||||||
|
* clears the source on every add/insert path — but the delta is the
|
||||||
|
* only event those paths emit, so the label corrects itself here or
|
||||||
|
* not at all.
|
||||||
|
*/
|
||||||
|
describe('queue store: the source travels on the delta', () => {
|
||||||
|
function syncWithAlbum(): void {
|
||||||
|
emit(Events.QueueChanged, {
|
||||||
|
tracks: [track(1), track(2)],
|
||||||
|
currentIndex: 0,
|
||||||
|
shuffleMode: false,
|
||||||
|
repeatMode: 'off',
|
||||||
|
source: { type: 'album', id: 7, label: 'Abbey Road' },
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
syncWithAlbum();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('drops the label when an append clears it backend-side', () => {
|
||||||
|
emit(Events.QueueTracksModified, {
|
||||||
|
action: 'add',
|
||||||
|
tracks: [track(3)],
|
||||||
|
index: 2,
|
||||||
|
currentIndex: 0,
|
||||||
|
source: { type: '', id: 0, label: '' },
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(queueStore.getState().source).toEqual({
|
||||||
|
type: '',
|
||||||
|
id: 0,
|
||||||
|
label: '',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps a label the backend still reports, as on a removal', () => {
|
||||||
|
emit(Events.QueueTracksModified, {
|
||||||
|
action: 'remove',
|
||||||
|
positions: [1],
|
||||||
|
index: 0,
|
||||||
|
currentIndex: 0,
|
||||||
|
source: { type: 'album', id: 7, label: 'Abbey Road' },
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(queueStore.getState().source).toEqual({
|
||||||
|
type: 'album',
|
||||||
|
id: 7,
|
||||||
|
label: 'Abbey Road',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('queue store: mode deltas', () => {
|
describe('queue store: mode deltas', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
sync([track(1)], 0);
|
sync([track(1)], 0);
|
||||||
|
|||||||
Reference in New Issue
Block a user