Repository navigation
fix: escape webUrl in Rich output, reconcile now reports what it actually compared - #26
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prompted by a full recheck of the repo for bugs/misleading claims/scope of improvement, requested directly.
Real bug fixed: cli.py's cloud-sync webUrl display (two call sites — 'export' and 'sync-cloud') interpolated a live Microsoft Graph API response field directly into Rich markup, unescaped. Same bug class fixed everywhere else in #23 (Rich parses literal brackets as markup and silently drops unrecognized tags) — missed these two call sites at the time.
Misleading-output fix: 'tendertracker reconcile' printed 'No discrepancies found' identically whether something was actually verified in sync, or nothing was available to compare against at all (Excel never exported, Pipedrive not configured) — a repo in that state would report a falsely clean result. reconcile() now tracks and reports excel_available/pipedrive_configured; the CLI explicitly says when nothing was actually compared.
Documentation fix: README's Testing section claimed reconciliation orchestration tests covered 'dry-run-vs-apply, idempotency, and the create-then-update transition' — those concepts don't apply to reconcile (it's always read-only, no apply flag). Corrected to describe what's actually tested there.
Documented (not silently changed) edge case: reconcile's Pipedrive value-drift check treats deal value=0 the same as unset/None — added an explicit comment on why (unverified against a live Pipedrive account which behavior is more correct) rather than guessing at a 'fix' I can't verify.
Verified: 56/56 tests pass (55 existing + 1 new regression test for the excel-unavailable case); manually confirmed the new reconcile messaging in both the 'nothing configured' and 'excel available' states.