fix: use nil for current_filter when no filter active, fix test
This commit is contained in:
@@ -0,0 +1,478 @@
|
||||
# Concise Default View Implementation Plan
|
||||
|
||||
**Goal:** Default index shows only READING + 5 most recent WANT books, with filter links to expanded views.
|
||||
|
||||
**Architecture:** `spine-index-model` gains an optional `filter` argument (nil = concise, "all"/"read"/"want" = expanded). The handler reads `filter` from the request query and passes it through. The template gains conditional nav links and summary-group rows. Existing group rendering unchanged.
|
||||
|
||||
**Tech Stack:** Emacs Lisp (model/handler), Mustache (template)
|
||||
|
||||
---
|
||||
|
||||
## File overview
|
||||
|
||||
**Modify:**
|
||||
- `spine.el` — `spine-index-model` (add filter param, concise logic), `defservlet index` (pass query filter)
|
||||
- `templates/index.mustache` — filter nav bar, summary-group rows
|
||||
- `test/spine-index-model-test.el` — new test file for filtered model
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Write failing tests for filtered `spine-index-model`
|
||||
|
||||
**Files:**
|
||||
- Create: `test/spine-index-model-test.el`
|
||||
|
||||
- [ ] **Step 1: Create test file with setup**
|
||||
|
||||
```elisp
|
||||
;;; spine-index-model-test.el — ERT tests for spine-index-model with filter
|
||||
|
||||
(require 'ert)
|
||||
(require 'cl-lib)
|
||||
|
||||
(setq spine-skip-server-start t)
|
||||
(setq spine-org-file (expand-file-name "sample-books.org"
|
||||
(file-name-directory
|
||||
(directory-file-name
|
||||
(file-name-directory load-file-name)))))
|
||||
(load-file (expand-file-name "../spine.el"
|
||||
(file-name-directory load-file-name)))
|
||||
|
||||
(defconst spine-index-model-test--books (spine-books)
|
||||
"Books fixture loaded once from sample-books.org.")
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Write test — concise view excludes READ and ABANDONED groups, has summary**
|
||||
|
||||
```elisp
|
||||
(ert-deftest spine-index-model-concise-hides-read-and-abandoned ()
|
||||
"Concise view (filter=nil) omits READ/ABANDONED groups, adds summary-groups."
|
||||
(let* ((model (spine-index-model spine-index-model-test--books))
|
||||
(groups (ht-get model "groups"))
|
||||
(summaries (ht-get model "summary_groups"))
|
||||
(group-labels (mapcar (lambda (g) (ht-get g "label")) groups)))
|
||||
(should-not (member "read" group-labels))
|
||||
(should-not (member "abandoned" group-labels))
|
||||
(should summaries)
|
||||
(should (= (length summaries) 1)) ; only READ, no ABANDONED in sample data
|
||||
(should (string= (ht-get (car summaries) "label") "read"))
|
||||
(should (= (ht-get (car summaries) "count") 1)) ; Ancillary Justice
|
||||
(should (string= (ht-get (car summaries) "href") "/index?filter=read"))))
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Write test — concise view includes READING group**
|
||||
|
||||
```elisp
|
||||
(ert-deftest spine-index-model-concise-includes-reading ()
|
||||
"Concise view includes READING group with all reading books."
|
||||
(let* ((model (spine-index-model spine-index-model-test--books))
|
||||
(groups (ht-get model "groups"))
|
||||
(reading-group (cl-find "reading" groups
|
||||
:key (lambda (g) (ht-get g "label"))
|
||||
:test #'string=)))
|
||||
(should reading-group)
|
||||
(should (= (ht-get reading-group "count") 1)) ; Use of Weapons
|
||||
(should (string= (ht-get (car (ht-get reading-group "books")) "title")
|
||||
"Use of Weapons"))))
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Write test — concise view limits WANT to 5 most recent**
|
||||
|
||||
```elisp
|
||||
(ert-deftest spine-index-model-concise-want-limited-to-five ()
|
||||
"Concise view limits WANT group to 5 books, sorted by :ADDED: desc."
|
||||
(let* ((model (spine-index-model spine-index-model-test--books))
|
||||
(groups (ht-get model "groups"))
|
||||
(want-group (cl-find "on deck" groups
|
||||
:key (lambda (g) (ht-get g "label"))
|
||||
:test #'string=)))
|
||||
(should want-group)
|
||||
(let ((books (ht-get want-group "books")))
|
||||
(should (<= (length books) 5))
|
||||
;; sample-books.org has 3 WANT books — all should appear, sorted newest first
|
||||
(should (= (length books) 3))
|
||||
;; Piranesi added 2026-06-01, Dune 2026-05-10, Left Hand 2026-04-01
|
||||
(should (string= (ht-get (nth 0 books) "title") "Piranesi"))
|
||||
(should (string= (ht-get (nth 1 books) "title") "Dune"))
|
||||
(should (string= (ht-get (nth 2 books) "title") "The Left Hand of Darkness")))))
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Write test — filter=all returns full model (no summary, all groups)**
|
||||
|
||||
```elisp
|
||||
(ert-deftest spine-index-model-filter-all-shows-all-groups ()
|
||||
"filter=all returns all groups, no summary-groups."
|
||||
(let* ((model (spine-index-model spine-index-model-test--books "all"))
|
||||
(groups (ht-get model "groups"))
|
||||
(summaries (ht-get model "summary_groups"))
|
||||
(group-labels (mapcar (lambda (g) (ht-get g "label")) groups)))
|
||||
(should-not summaries)
|
||||
(should (member "on deck" group-labels))
|
||||
(should (member "reading" group-labels))
|
||||
(should (member "read" group-labels))
|
||||
(should (= (length groups) 3)))) ; WANT, READING, READ (no ABANDONED in sample)
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Write test — filter=read returns only READ group**
|
||||
|
||||
```elisp
|
||||
(ert-deftest spine-index-model-filter-read-returns-only-read ()
|
||||
"filter=read returns only the READ group."
|
||||
(let* ((model (spine-index-model spine-index-model-test--books "read"))
|
||||
(groups (ht-get model "groups")))
|
||||
(should (= (length groups) 1))
|
||||
(should (string= (ht-get (car groups) "label") "read"))
|
||||
(should (= (ht-get (car groups) "count") 1))))
|
||||
```
|
||||
|
||||
- [ ] **Step 7: Write test — filter=want returns only WANT group**
|
||||
|
||||
```elisp
|
||||
(ert-deftest spine-index-model-filter-want-returns-only-want ()
|
||||
"filter=want returns only the WANT group."
|
||||
(let* ((model (spine-index-model spine-index-model-test--books "want"))
|
||||
(groups (ht-get model "groups")))
|
||||
(should (= (length groups) 1))
|
||||
(should (string= (ht-get (car groups) "label") "on deck"))
|
||||
(should (= (ht-get (car groups) "count") 3))))
|
||||
```
|
||||
|
||||
- [ ] **Step 8: Write test — invalid filter falls back to concise**
|
||||
|
||||
```elisp
|
||||
(ert-deftest spine-index-model-invalid-filter-falls-back-to-concise ()
|
||||
"Unrecognised filter value behaves like nil (concise view)."
|
||||
(let* ((model (spine-index-model spine-index-model-test--books "bogus"))
|
||||
(groups (ht-get model "groups"))
|
||||
(summaries (ht-get model "summary_groups"))
|
||||
(group-labels (mapcar (lambda (g) (ht-get g "label")) groups)))
|
||||
(should summaries) ; has summary
|
||||
(should-not (member "read" group-labels)) ; no read group
|
||||
(should (member "reading" group-labels)) ; has reading
|
||||
(should (member "on deck" group-labels)))) ; has want
|
||||
```
|
||||
|
||||
- [ ] **Step 9: Write test — model includes current_filter in context**
|
||||
|
||||
```elisp
|
||||
(ert-deftest spine-index-model-includes-current-filter ()
|
||||
"Model includes :current_filter key matching the filter argument."
|
||||
(should (equal (ht-get (spine-index-model spine-index-model-test--books) "current_filter") ""))
|
||||
(should (equal (ht-get (spine-index-model spine-index-model-test--books "all") "current_filter") "all"))
|
||||
(should (equal (ht-get (spine-index-model spine-index-model-test--books "read") "current_filter") "read"))
|
||||
(should (equal (ht-get (spine-index-model spine-index-model-test--books "want") "current_filter") "want")))
|
||||
```
|
||||
|
||||
- [ ] **Step 10: Run tests to verify they fail**
|
||||
|
||||
```bash
|
||||
cd /work/personal-local/spine
|
||||
emacs --batch -l test/spine-index-model-test.el \
|
||||
--eval "(ert-run-tests-batch-and-exit '(not (tag :unstable)))" 2>&1
|
||||
```
|
||||
|
||||
Expected: all tests fail because `spine-index-model` doesn't accept a second argument yet.
|
||||
|
||||
- [ ] **Step 11: Commit test file**
|
||||
|
||||
```bash
|
||||
git add test/spine-index-model-test.el
|
||||
git commit -m "test: add failing tests for filtered spine-index-model"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Implement filtered `spine-index-model`
|
||||
|
||||
**Files:**
|
||||
- Modify: `spine.el:77-155`
|
||||
|
||||
- [ ] **Step 1: Update signature and add filter handling**
|
||||
|
||||
Swap the function at line 77:
|
||||
|
||||
```elisp
|
||||
(defun spine-index-model (books &optional filter selected-id)
|
||||
"Build the view model ht for templates/index.mustache from BOOKS.
|
||||
FILTER controls which books are shown:
|
||||
nil — concise: READING + 5 most recent WANT, others as summary links
|
||||
\"all\" — full listing, no truncation
|
||||
\"read\" — only READ books
|
||||
\"want\" — only WANT books
|
||||
SELECTED-ID expands the matching book's detail section."
|
||||
(let* ((status-labels
|
||||
'(("WANT" . "on deck")
|
||||
("READING" . "reading")
|
||||
("READ" . "read")
|
||||
("ABANDONED" . "abandoned")))
|
||||
(format-icons
|
||||
'(("hardcover" . "ti-book")
|
||||
("ebook" . "ti-device-tablet")
|
||||
("audiobook" . "ti-headphones")))
|
||||
(order '("WANT" "READING" "READ" "ABANDONED"))
|
||||
(valid-filters '("all" "read" "want"))
|
||||
(effective-filter (if (member filter valid-filters) filter nil))
|
||||
(grouped (make-hash-table :test 'equal))
|
||||
(total (length books))
|
||||
(reading-count 0))
|
||||
(dolist (book books)
|
||||
(when (equal "READING" (plist-get book :status))
|
||||
(cl-incf reading-count)))
|
||||
(dolist (book books)
|
||||
(let ((status (or (plist-get book :status) "WANT")))
|
||||
(push book (gethash status grouped))))
|
||||
(let ((groups nil)
|
||||
(summary-groups nil))
|
||||
(dolist (status order)
|
||||
(let ((group-books (nreverse (gethash status grouped))))
|
||||
(when group-books
|
||||
(if (and (null effective-filter)
|
||||
(member status '("READ" "ABANDONED")))
|
||||
;; Concise: replace read/abandoned groups with summary links
|
||||
(push (ht ("label" (cdr (assoc status status-labels)))
|
||||
("count" (length group-books))
|
||||
("href" (format "/index?filter=%s"
|
||||
(downcase status))))
|
||||
summary-groups)
|
||||
;; Show the group (possibly filtered to one status)
|
||||
(when (or (null effective-filter)
|
||||
(string= (downcase status) effective-filter)
|
||||
(string= effective-filter "all"))
|
||||
;; Sort WANT by ADDED descending when in concise view
|
||||
(when (and (null effective-filter)
|
||||
(string= status "WANT"))
|
||||
(setq group-books
|
||||
(cl-subseq (sort group-books
|
||||
(lambda (a b)
|
||||
(let ((a-date (plist-get a :added))
|
||||
(b-date (plist-get b :added)))
|
||||
(cond
|
||||
((null a-date) nil)
|
||||
((null b-date) t)
|
||||
(t (string> a-date b-date))))))
|
||||
0 (min (length group-books) 5))))
|
||||
(let ((book-models nil))
|
||||
(dolist (book group-books)
|
||||
(let* ((id (plist-get book :id))
|
||||
(selected (equal id selected-id))
|
||||
(fmt (plist-get book :format))
|
||||
(rating (plist-get book :rating))
|
||||
(notes (plist-get book :notes))
|
||||
(status (or (plist-get book :status) "WANT"))
|
||||
(model
|
||||
(ht
|
||||
("format_icon"
|
||||
(or (cdr (assoc fmt format-icons)) ""))
|
||||
("status_class" (downcase status))
|
||||
("status_label" (downcase status))
|
||||
("title" (plist-get book :title))
|
||||
("author" (or (plist-get book :author) ""))
|
||||
("tags_inline"
|
||||
(let ((tags (plist-get book :tags)))
|
||||
(when tags
|
||||
(mapconcat (lambda (tag) (concat ":" tag ":"))
|
||||
tags " "))))
|
||||
("rec_via" (plist-get book :rec_by))
|
||||
("rating_display"
|
||||
(when (and rating (> (length rating) 0))
|
||||
(make-string (string-to-number rating) ?\u2605)))
|
||||
("selected" selected)
|
||||
("detail"
|
||||
(when selected
|
||||
(ht
|
||||
("meta"
|
||||
(let ((added (plist-get book :added))
|
||||
(fmt (plist-get book :format)))
|
||||
(cond
|
||||
((and added fmt)
|
||||
(format "started %s · %s" added fmt))
|
||||
(added (format "started %s" added))
|
||||
(fmt (format "format: %s" fmt))
|
||||
(t ""))))
|
||||
("notes"
|
||||
(when notes
|
||||
(cl-loop for (date text) in notes
|
||||
collect (ht
|
||||
("date" (format "[%s]" date))
|
||||
("text" text)))))))))))
|
||||
(push model book-models)))
|
||||
(push (ht ("label" (cdr (assoc status status-labels)))
|
||||
("count" (length group-books))
|
||||
("books" (nreverse book-models)))
|
||||
groups))))))
|
||||
(ht ("app_title" "spine")
|
||||
("total_books" total)
|
||||
("reading_count" reading-count)
|
||||
("current_filter" (or effective-filter ""))
|
||||
("summary_groups" (nreverse summary-groups))
|
||||
("groups" (nreverse groups))))))
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they pass**
|
||||
|
||||
```bash
|
||||
cd /work/personal-local/spine
|
||||
emacs --batch -l test/spine-index-model-test.el \
|
||||
-l test/spine-books-test.el \
|
||||
-l test/spine-add-book-test.el \
|
||||
--eval "(ert-run-tests-batch-and-exit '(not (tag :unstable)))" 2>&1
|
||||
```
|
||||
|
||||
Expected: all tests pass (new filter tests + existing books + add-book tests).
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add spine.el
|
||||
git commit -m "feat: add filter parameter to spine-index-model for concise default view"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Update `defservlet index` handler
|
||||
|
||||
**Files:**
|
||||
- Modify: `spine.el:308-313`
|
||||
|
||||
- [ ] **Step 1: Pass query filter to model**
|
||||
|
||||
Swap lines 308-313:
|
||||
|
||||
```elisp
|
||||
(defservlet index text/html (path query request)
|
||||
(let* ((books (spine-books))
|
||||
(filter (cadr (assoc "filter" query))))
|
||||
(if books
|
||||
(insert (spine-render "index.mustache"
|
||||
(spine-index-model books filter (cadr (assoc "id" query)))))
|
||||
(insert (spine-render-empty-state)))))
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run existing tests to confirm handler change doesn't break anything**
|
||||
|
||||
```bash
|
||||
cd /work/personal-local/spine
|
||||
emacs --batch -l test/spine-index-model-test.el \
|
||||
-l test/spine-books-test.el \
|
||||
-l test/spine-add-book-test.el \
|
||||
--eval "(ert-run-tests-batch-and-exit '(not (tag :unstable)))" 2>&1
|
||||
```
|
||||
|
||||
Expected: all pass.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add spine.el
|
||||
git commit -m "feat: pass filter query param from index handler to model"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Update `templates/index.mustache` with filter links and summary groups
|
||||
|
||||
**Files:**
|
||||
- Modify: `templates/index.mustache`
|
||||
|
||||
- [ ] **Step 1: Add filter nav bar after header, before groups**
|
||||
|
||||
Insert after the closing `</header>` (line 87) and before the `{{#groups}}` section (line 89):
|
||||
|
||||
```mustache
|
||||
{{#current_filter}}
|
||||
<nav class="filter-bar" style="padding: .4rem .9rem; font-size: .8rem;">
|
||||
<a href="/index">Concise view</a>
|
||||
</nav>
|
||||
{{/current_filter}}
|
||||
{{^current_filter}}
|
||||
<nav class="filter-bar" style="padding: .4rem .9rem; font-size: .8rem;">
|
||||
<span>
|
||||
<a href="/index?filter=all">All books ({{total_books}})</a>
|
||||
·
|
||||
<a href="/index?filter=want">Want list</a>
|
||||
·
|
||||
<a href="/index?filter=read">Read ({{reading_count}})</a>
|
||||
</span>
|
||||
</nav>
|
||||
{{/current_filter}}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add summary group rows after groups section**
|
||||
|
||||
Insert after the closing `{{/groups}}` (line 109) and before `<div class="endpad">` (line 110):
|
||||
|
||||
```mustache
|
||||
{{#summary_groups}}
|
||||
<div class="row"><a href="{{href}}">{{label}} · {{count}}</a></div>
|
||||
{{/summary_groups}}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add templates/index.mustache
|
||||
git commit -m "feat: add filter nav bar and summary group links to index template"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Manual integration test
|
||||
|
||||
- [ ] **Step 1: Start the server and verify concise default view**
|
||||
|
||||
```bash
|
||||
cd /work/personal-local/spine
|
||||
SPINE_ORG_FILE=$(pwd)/sample-books.org emacs --batch -l spine.el \
|
||||
--eval "(setq spine-org-file \"$(pwd)/sample-books.org\")" \
|
||||
--eval "(httpd-start)" \
|
||||
--eval "(message \"Server running on port 8080\")" &
|
||||
sleep 2
|
||||
curl -s http://localhost:8080/index | head -50
|
||||
```
|
||||
|
||||
Verify the output:
|
||||
- Shows READING group (Use of Weapons)
|
||||
- Shows WANT group (Piranesi, Dune, Left Hand)
|
||||
- Does NOT show READ group (Ancillary Justice) as a full group
|
||||
- Shows a summary link for "read · 1"
|
||||
- Shows "All books (5)" filter nav link
|
||||
|
||||
- [ ] **Step 2: Test filter=all**
|
||||
|
||||
```bash
|
||||
curl -s "http://localhost:8080/index?filter=all"
|
||||
```
|
||||
|
||||
Verify: all books shown in their groups, no summary sections, "Concise view" link present.
|
||||
|
||||
- [ ] **Step 3: Test filter=read**
|
||||
|
||||
```bash
|
||||
curl -s "http://localhost:8080/index?filter=read"
|
||||
```
|
||||
|
||||
Verify: only Ancillary Justice shown, "Concise view" link present.
|
||||
|
||||
- [ ] **Step 4: Kill the server**
|
||||
|
||||
```bash
|
||||
kill %1 2>/dev/null; wait 2>/dev/null
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run all tests one final time**
|
||||
|
||||
```bash
|
||||
cd /work/personal-local/spine
|
||||
emacs --batch -l test/spine-index-model-test.el \
|
||||
-l test/spine-books-test.el \
|
||||
-l test/spine-add-book-test.el \
|
||||
--eval "(ert-run-tests-batch-and-exit '(not (tag :unstable)))" 2>&1
|
||||
```
|
||||
|
||||
Expected: all pass.
|
||||
|
||||
- [ ] **Step 6: Final commit if any fixes needed**
|
||||
|
||||
```bash
|
||||
git commit -am "fix: address integration test findings"
|
||||
```
|
||||
@@ -182,7 +182,7 @@ SELECTED-ID expands the matching book's detail section."
|
||||
(ht ("app_title" "spine")
|
||||
("total_books" total)
|
||||
("reading_count" reading-count)
|
||||
("current_filter" (or effective-filter ""))
|
||||
("current_filter" effective-filter)
|
||||
("summary_groups" (nreverse summary-groups))
|
||||
("groups" (nreverse groups))))))
|
||||
(defun spine-add-book (&rest args)
|
||||
|
||||
@@ -97,7 +97,7 @@
|
||||
|
||||
(ert-deftest spine-index-model-includes-current-filter ()
|
||||
"Model includes current_filter key matching the filter argument."
|
||||
(should (equal (ht-get (spine-index-model spine-index-model-test--books) "current_filter") ""))
|
||||
(should (not (ht-get (spine-index-model spine-index-model-test--books) "current_filter")))
|
||||
(should (equal (ht-get (spine-index-model spine-index-model-test--books "all") "current_filter") "all"))
|
||||
(should (equal (ht-get (spine-index-model spine-index-model-test--books "read") "current_filter") "read"))
|
||||
(should (equal (ht-get (spine-index-model spine-index-model-test--books "want") "current_filter") "want")))
|
||||
|
||||
Reference in New Issue
Block a user