diff options
| author | Craig Jennings <c@cjennings.net> | 2026-05-28 10:24:21 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-05-28 10:24:21 -0500 |
| commit | e5a07739c5b58b041bcb7192eae35a2c4d98bce3 (patch) | |
| tree | a424567f5c6acef8ce7d0b2494c3cb250abf8e08 | |
| parent | 7939a598bc1f2e045dd4d4fe9e5dc4b7437147d5 (diff) | |
| download | pearl-e5a07739c5b58b041bcb7192eae35a2c4d98bce3.tar.gz pearl-e5a07739c5b58b041bcb7192eae35a2c4d98bce3.zip | |
refactor(view-sync): pick-scope returns (display . plist), drops rassoc lookup
pearl-sync-saved-query-to-linear used to rebuild the entire scope-candidates list a second time (full sort + 2N row allocations) just to rassoc the chosen scope plist back to its display string for the success message. The picker had thrown away the display string the user picked, so the caller had no choice but to re-derive it.
I changed pearl--sync-saved-query-pick-scope to return (DISPLAY . PLIST) instead of just PLIST. The caller now reads scope-label from (car scope-pair) and scope from (cdr scope-pair). The rassoc is gone, and so is the second pearl--sync-scope-candidates call.
To keep the re-sync path label-consistent with the picker (so a "Synced X (Team: Engineering, visible to the team)" message matches what the picker would have shown for the equivalent first-time sync), I extracted pearl--sync-scope-label as the single source of truth for the label format. The picker, the re-sync branch, and any future caller that needs to describe a scope all go through the helper. Personal scope always renders "[ Personal, only I see it ]" regardless of the shared flag (a Personal + shared combination is meaningless on Linear). Team scopes render "[ Team: NAME, visible to the team ]" or "[ Team: NAME, only I see it ]" per the flag.
I added five tests covering the new behavior: scope-label for personal (ignores shared) and team (both shared and private), pick-scope returns the team pair, pick-scope returns the personal pair, pick-scope returns nil on cancel. All 668 ert tests pass. make compile and make lint are clean.
| -rw-r--r-- | pearl.el | 145 | ||||
| -rw-r--r-- | tests/test-pearl-saved-query-sync.el | 83 |
2 files changed, 162 insertions, 66 deletions
@@ -3738,6 +3738,18 @@ stored when given." ;;; Saved-query -> Linear view sync (see docs/saved-query-sync-spec.org) +(defun pearl--sync-scope-label (team-name shared) + "Return the display label for the scope identified by TEAM-NAME and SHARED. +TEAM-NAME is either \"Personal\" or a real team name. SHARED is t +\(visible to the team) or nil (only the viewer). The single source of +truth so the picker, the re-sync path, and the success message all +agree byte-for-byte." + (if (equal team-name "Personal") + "[ Personal, only I see it ]" + (format "[ Team: %s, %s ]" + team-name + (if shared "visible to the team" "only I see it")))) + (defun pearl--sync-scope-candidates (teams filter-team-key) "Build the (DISPLAY . PLIST) candidate list for the scope-and-visibility prompt. TEAMS is the team alist list returned by `pearl--all-teams' (each carries @@ -3760,7 +3772,7 @@ The meaningless `Personal + shared' combination is absent." (equal filter-team-key (cdr (assoc 'key tm)))) sorted-teams))) (personal-row - (cons "[ Personal, only I see it ]" + (cons (pearl--sync-scope-label "Personal" nil) (list :team-id nil :team-name "Personal" :shared nil))) (team-rows (cl-mapcan @@ -3768,16 +3780,15 @@ The meaningless `Personal + shared' combination is absent." (let ((name (cdr (assoc 'name tm))) (id (cdr (assoc 'id tm)))) (list - (cons (format "[ Team: %s, visible to the team ]" name) + (cons (pearl--sync-scope-label name t) (list :team-id id :team-name name :shared t)) - (cons (format "[ Team: %s, only I see it ]" name) + (cons (pearl--sync-scope-label name nil) (list :team-id id :team-name name :shared nil))))) sorted-teams)) (all-rows (cons personal-row team-rows)) (default-display (if filter-team - (format "[ Team: %s, visible to the team ]" - (cdr (assoc 'name filter-team))) + (pearl--sync-scope-label (cdr (assoc 'name filter-team)) t) (car personal-row))) (default-row (cl-find default-display all-rows :key #'car :test #'string=))) @@ -3873,9 +3884,11 @@ local link couldn't be saved." (defun pearl--sync-saved-query-pick-scope (filter-plist) "Prompt the user for the destination scope of a sync-up. FILTER-PLIST is the saved query's authoring filter (used to derive the -default candidate). Returns the chosen scope plist -`(:team-id ID-OR-NIL :team-name NAME :shared BOOL)', or nil when the -user cancels via the cancel sentinel." +default candidate). Returns `(DISPLAY . PLIST)', or nil when the user +cancels via the cancel sentinel. DISPLAY is the candidate string the +user picked; PLIST is `(:team-id ID-OR-NIL :team-name NAME :shared +BOOL)'. Returning both halves saves the caller from rebuilding the +candidate list to look the display string back up via rassoc." (let* ((teams (pearl--all-teams)) (cands (pearl--sync-scope-candidates teams (plist-get filter-plist :team))) (displays (mapcar #'car cands)) @@ -3885,7 +3898,7 @@ user cancels via the cancel sentinel." (pearl--with-sentinel pearl--filter-cancel displays)) nil t))) (unless (pearl--filter-sentinel-value-p choice) - (cdr (assoc choice cands))))) + (assoc choice cands)))) (defun pearl--sync-saved-query-collision-action (name) "Prompt the user when a view named NAME already exists in the chosen scope. @@ -3963,64 +3976,72 @@ the user can re-sync with Replace to reconcile." (user-error "Saved query %s has no filter constraints; Linear views need at least one constraint -- add one and re-sync" name)) - (let ((scope (if existing-view-id - ;; Re-sync uses the stored scope. - (list :team-id existing-team-id - :team-name (or (and existing-team-id - (cdr (assoc 'name - (seq-find - (lambda (tm) - (equal existing-team-id - (cdr (assoc 'id tm)))) - (pearl--all-teams))))) - "Personal") - :shared existing-shared) - (pearl--sync-saved-query-pick-scope filter-plist)))) - (unless scope + (let ((scope-pair + (if existing-view-id + ;; Re-sync: build the scope pair from stored metadata, + ;; resolving the team name from id (defaults to "Personal" + ;; when team-id is nil OR the team is no longer in the + ;; cache). The label comes from the same helper the + ;; picker uses so the success message matches what the + ;; user would have seen on first sync. + (let* ((team-name (or (and existing-team-id + (cdr (assoc 'name + (seq-find + (lambda (tm) + (equal existing-team-id + (cdr (assoc 'id tm)))) + (pearl--all-teams))))) + "Personal"))) + (cons (pearl--sync-scope-label team-name existing-shared) + (list :team-id existing-team-id + :team-name team-name + :shared existing-shared))) + ;; First-time sync: pick-scope returns (display . plist) or + ;; nil (cancel). + (pearl--sync-saved-query-pick-scope filter-plist)))) + (unless scope-pair (user-error "Cancelled; no saved query synced")) ;; Validate the filter at the boundary so a malformed entry surfaces ;; before the API call. (pearl--validate-issue-filter filter-plist) - (let* ((team-id (plist-get scope :team-id)) - (shared (plist-get scope :shared)) - (scope-label (car (rassoc scope - (pearl--sync-scope-candidates - (pearl--all-teams) - (plist-get filter-plist :team)))))) + (let* ((scope-label (car scope-pair)) + (scope (cdr scope-pair)) + (team-id (plist-get scope :team-id)) + (shared (plist-get scope :shared))) (cond - ;; First-time sync: check for a same-name collision in the chosen - ;; scope and either update-by-id (Replace), re-prompt (Rename), or - ;; bail (Cancel). - ((null existing-view-id) - (let* ((collision (pearl--find-view-by-name-in-scope - (pearl--custom-views t) - target-name team-id shared)) - (action (and collision - (pearl--sync-saved-query-collision-action - target-name)))) - (pcase action - ('cancel - (message "Cancelled; %s left unsynced" target-name)) - ('rename - (setq target-name - (read-string (format "New view name (was %s): " name))) - (when (or (null target-name) (string-empty-p target-name)) - (user-error "Cancelled; no name entered")) - (pearl--sync-saved-query-do-create - name target-name team-id shared filter-data scope-label)) - ('replace - (pearl--sync-saved-query-do-update - name (cdr (assoc 'id collision)) - team-id shared filter-data scope-label)) - (_ - ;; No collision -- straight create. - (pearl--sync-saved-query-do-create - name target-name team-id shared filter-data scope-label))))) - ;; Re-sync: update by stored id; no collision check (the id targets - ;; the existing view directly). - (t - (pearl--sync-saved-query-do-update - name existing-view-id team-id shared filter-data scope-label)))))))) + ;; First-time sync: check for a same-name collision in the chosen + ;; scope and either update-by-id (Replace), re-prompt (Rename), or + ;; bail (Cancel). + ((null existing-view-id) + (let* ((collision (pearl--find-view-by-name-in-scope + (pearl--custom-views t) + target-name team-id shared)) + (action (and collision + (pearl--sync-saved-query-collision-action + target-name)))) + (pcase action + ('cancel + (message "Cancelled; %s left unsynced" target-name)) + ('rename + (setq target-name + (read-string (format "New view name (was %s): " name))) + (when (or (null target-name) (string-empty-p target-name)) + (user-error "Cancelled; no name entered")) + (pearl--sync-saved-query-do-create + name target-name team-id shared filter-data scope-label)) + ('replace + (pearl--sync-saved-query-do-update + name (cdr (assoc 'id collision)) + team-id shared filter-data scope-label)) + (_ + ;; No collision -- straight create. + (pearl--sync-saved-query-do-create + name target-name team-id shared filter-data scope-label))))) + ;; Re-sync: update by stored id; no collision check (the id targets + ;; the existing view directly). + (t + (pearl--sync-saved-query-do-update + name existing-view-id team-id shared filter-data scope-label)))))))) (defun pearl--sync-saved-query-do-create (name target-name team-id shared filter-data scope-label) diff --git a/tests/test-pearl-saved-query-sync.el b/tests/test-pearl-saved-query-sync.el index 8c98539..e237108 100644 --- a/tests/test-pearl-saved-query-sync.el +++ b/tests/test-pearl-saved-query-sync.el @@ -110,6 +110,60 @@ (< apple-pos mango-pos) (< mango-pos zebra-pos))))) +;;; pearl--sync-scope-label + +(ert-deftest test-pearl-sync-scope-label-personal-ignores-shared () + "Personal scope always renders the personal label regardless of SHARED. +A `Personal + shared' combination is meaningless on Linear (there's no +team to share with), so the label collapses to the only sensible +phrasing." + (should (equal "[ Personal, only I see it ]" + (pearl--sync-scope-label "Personal" nil))) + (should (equal "[ Personal, only I see it ]" + (pearl--sync-scope-label "Personal" t)))) + +(ert-deftest test-pearl-sync-scope-label-team-renders-shared-or-private () + "A team scope renders the shared or private variant per SHARED." + (should (equal "[ Team: Engineering, visible to the team ]" + (pearl--sync-scope-label "Engineering" t))) + (should (equal "[ Team: Engineering, only I see it ]" + (pearl--sync-scope-label "Engineering" nil)))) + +(ert-deftest test-pearl-sync-saved-query-pick-scope-returns-display-plist-pair () + "`pick-scope' returns the chosen candidate's (DISPLAY . PLIST) pair, not +just the plist. The caller uses the display string for the success +message without rebuilding the candidate list to look it up." + (cl-letf (((symbol-function 'pearl--all-teams) + (lambda () '(((id . "t1") (key . "ENG") (name . "Engineering"))))) + ((symbol-function 'completing-read) + (lambda (&rest _) "[ Team: Engineering, visible to the team ]"))) + (let ((pair (pearl--sync-saved-query-pick-scope '(:team "ENG")))) + (should (consp pair)) + (should (equal "[ Team: Engineering, visible to the team ]" + (car pair))) + (should (equal "t1" (plist-get (cdr pair) :team-id))) + (should (eq t (plist-get (cdr pair) :shared)))))) + +(ert-deftest test-pearl-sync-saved-query-pick-scope-returns-personal-pair () + "Picking the Personal row returns the personal pair: display = the personal +label, plist team-id nil + team-name \"Personal\" + shared nil." + (cl-letf (((symbol-function 'pearl--all-teams) (lambda () '())) + ((symbol-function 'completing-read) + (lambda (&rest _) "[ Personal, only I see it ]"))) + (let ((pair (pearl--sync-saved-query-pick-scope '()))) + (should (consp pair)) + (should (equal "[ Personal, only I see it ]" (car pair))) + (should (null (plist-get (cdr pair) :team-id))) + (should (equal "Personal" (plist-get (cdr pair) :team-name))) + (should (null (plist-get (cdr pair) :shared)))))) + +(ert-deftest test-pearl-sync-saved-query-pick-scope-returns-nil-on-cancel () + "Picking the cancel sentinel returns nil so the caller can short-circuit." + (cl-letf (((symbol-function 'pearl--all-teams) (lambda () '())) + ((symbol-function 'completing-read) + (lambda (&rest _) pearl--filter-cancel))) + (should (null (pearl--sync-saved-query-pick-scope '()))))) + ;;; pearl--find-view-by-name-in-scope (ert-deftest test-pearl-find-view-by-name-in-scope-matches-team-shared () @@ -418,16 +472,20 @@ erased on every re-sync." ;;; pearl-sync-saved-query-to-linear — empty filter refusal (ert-deftest test-pearl-sync-empty-filter-refuses-before-scope-prompt () - "A saved query whose filter compiles to nil signals a clear user-error -before any scope prompt fires. Without the guard, the sync would pass -filterData=null to Linear, which rejects with an opaque error after the + "A first-time-sync saved query whose filter compiles to nil signals a clear +user-error before any scope prompt fires. Without the guard, the sync would +pass filterData=null to Linear, which rejects with an opaque error after the user has already clicked through the scope picker." (let ((pearl-saved-queries (copy-tree '(("EmptyFilter" :filter nil)))) (scope-prompt-fired nil) (api-called nil)) (cl-letf (((symbol-function 'pearl--sync-saved-query-pick-scope) (lambda (&rest _) (setq scope-prompt-fired t) - (list :team-id nil :team-name "Personal" :shared nil))) + ;; Pick-scope returns (DISPLAY . PLIST); the empty-filter + ;; guard fires before this is consumed, but the stub + ;; shape should still match production. + (cons "[ Personal, only I see it ]" + (list :team-id nil :team-name "Personal" :shared nil)))) ((symbol-function 'pearl--customview-create-async) (lambda (&rest _) (setq api-called t)))) (should-error (pearl-sync-saved-query-to-linear "EmptyFilter") @@ -435,6 +493,23 @@ user has already clicked through the scope picker." (should-not scope-prompt-fired) (should-not api-called)))) +(ert-deftest test-pearl-sync-empty-filter-refuses-resync-too () + "A re-sync (entry already has `:linear-view-id') whose filter has been +edited down to nil takes the same empty-filter guard, refusing without +calling customViewUpdate. Guards against a future refactor accidentally +hoisting the guard into the create-only branch." + (let ((pearl-saved-queries + (copy-tree '(("EmptyFilter" :filter nil + :linear-view-id "existing-view-uuid" + :linear-view-team-id nil + :linear-view-shared :json-false)))) + (update-called nil)) + (cl-letf (((symbol-function 'pearl--customview-update-async) + (lambda (&rest _) (setq update-called t)))) + (should-error (pearl-sync-saved-query-to-linear "EmptyFilter") + :type 'user-error) + (should-not update-called)))) + ;;; pearl-publish-current-source (defmacro test-pearl-publish--in-buffer (source-form &rest body) |
