Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle filter bys for invalid dates/empty #10097

Merged
merged 4 commits into from
May 20, 2024

Conversation

joaoviana
Copy link
Contributor

Closes: #10096

Description:

Steps to reproduce:

  • have empty dates on a csv for the seed data
  • try to filter to/by on an explore or on a dashboard
  • it breaks the explore (see ticket with the Sentry replay)

Fix for a chart

fix-chart.mov

Fix for a dashboard

fix-dashboard.mov

Reviewer actions

  • I have manually tested the changes in the preview environment
  • I have reviewed the code
  • I understand that "request changes" will block this PR from merging

@joaoviana joaoviana requested a review from magnew May 16, 2024 17:05
@joaoviana joaoviana self-assigned this May 16, 2024
@owlas owlas requested a deployment to fix/filter-to-by-invalid-date - jaffle_db_pg_13 PR #10097 May 16, 2024 17:05 — with Render Abandoned
@owlas owlas deployed to fix/filter-to-by-invalid-date - headless-browser PR #10097 May 16, 2024 17:05 — with Render Active
Copy link

netlify bot commented May 16, 2024

Deploy Preview for peaceful-bassi-cbf284 canceled.

Name Link
🔨 Latest commit 3b50432
🔍 Latest deploy log https://app.netlify.com/sites/peaceful-bassi-cbf284/deploys/664b0b151ea50c0008989e73

Copy link
Contributor

@magnew magnew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work!

Left a suggestion

value: any,
) =>
isDimension(item) &&
Object.values(TimeFrames).includes(item.timeInterval as TimeFrames) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just checking its a date type? Could you use isDateItem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes... my intellisense just wasn't working 😥 thank you!

@owlas owlas deployed to fix/filter-to-by-invalid-date - headless-browser PR #10097 May 19, 2024 17:08 — with Render Active
@joaoviana joaoviana force-pushed the fix/filter-to-by-invalid-date branch from e142e52 to 2f8b1cf Compare May 20, 2024 08:22
@owlas owlas deployed to fix/filter-to-by-invalid-date - headless-browser PR #10097 May 20, 2024 08:22 — with Render Active
@owlas owlas temporarily deployed to fix/filter-to-by-invalid-date - headless-browser PR #10097 May 20, 2024 08:34 — with Render Destroyed
@owlas owlas temporarily deployed to fix/filter-to-by-invalid-date - lightdash PR #10097 May 20, 2024 08:34 — with Render Destroyed
@joaoviana joaoviana merged commit 2c3a193 into main May 20, 2024
47 of 52 checks passed
@joaoviana joaoviana deleted the fix/filter-to-by-invalid-date branch May 20, 2024 10:17
lightdash-bot pushed a commit that referenced this pull request May 20, 2024
## [0.1097.1](0.1097.0...0.1097.1) (2024-05-20)

### Bug Fixes

* handle filter bys for invalid dates/empty ([#10097](#10097)) ([2c3a193](2c3a193))
@lightdash-bot
Copy link
Collaborator

🎉 This PR is included in version 0.1097.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter by... menu action when field is Date and value is null leads to error
4 participants