diff options
| -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) |