Skip to content

fix: canvas custom comparison not applying - #9897

Merged
AdityaHegde merged 2 commits into
mainfrom
fix/canvas-custom-comparison
Sep 17, 2026
Merged

AdityaHegde merged 2 commits into
mainfrom
fix/canvas-custom-comparison

Conversation

@AdityaHegde

@AdityaHegde AdityaHegde commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Canvas custom comparison option was non functional.

  1. Min/max dates were swapped, leading to invalid interval.
  2. Custom range was considered as new rill time and ignored in comparison time.

Note that this is a minimal set of changes. There is a massive refactor coming to time controls that will add tests etc.

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@AdityaHegde AdityaHegde added Type:Bug Something isn't working Size:S Small change: under 100 lines Team:Applications Applications Working Group labels Sep 16, 2026
@nishantmonu51

Copy link
Copy Markdown
Collaborator

isNewRillTimeFormat now returns false for new-syntax absolute ranges (parser.ts:17, RillTime.ts:49)

2024-01-01 to 2024-02-01, 2024-01-01/2024-02-01 and 2024-01-01,2024-02-01 all parse to RillIsoInterval — new format by the name's own reading — yet now report false. The one caller (comparisons/index.ts:325) is really asking "must I resolve this expression instead of computing the range here", which is not the same question, and a later reader has no way to tell absolute ranges are excluded too.

isAbsolute duplicates isAbsoluteTime() (RillTime.ts:49, RillTime.ts:103)

Same instanceof RillIsoInterval check, now in two places to keep in sync. Calling parser.isAbsoluteTime() from isNewRillTimeFormat drops the field; renaming it to state what it means (isRillTimeExpression) makes the call site self-explanatory.

Test (comparisons/index.spec.ts:124)

A case for "2024-01-01T00:00:00.000Z,2024-02-01T00:00:00.000Z" in getComparisonIntervalTests pins this regression. The adjacent invalidComparisionTests covers the new-syntax rejections but not the absolute-range acceptance.

@AdityaHegde
AdityaHegde merged commit d296533 into main Sep 17, 2026
10 checks passed
@AdityaHegde
AdityaHegde deleted the fix/canvas-custom-comparison branch September 17, 2026 05:28
nishantmonu51 pushed a commit that referenced this pull request Sep 17, 2026
* fix: canvas custom comparison not applying

* PR review

(cherry picked from commit d296533)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size:S Small change: under 100 lines Team:Applications Applications Working Group Type:Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants