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

Filtering "Exclude" shows wrong weekday #21979

Closed
flamber opened this issue Apr 23, 2022 · 6 comments · Fixed by #22135
Closed

Filtering "Exclude" shows wrong weekday #21979

flamber opened this issue Apr 23, 2022 · 6 comments · Fixed by #22135
Assignees
Labels
.Correctness .Frontend Priority:P2 Average run of the mill bug Querying/Parameters & Variables Filter widgets, field filters, variables etc. .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Milestone

Comments

@flamber
Copy link
Contributor

flamber commented Apr 23, 2022

Describe the bug
Filtering "Exclude" shows wrong weekend - though, the actual query is correct - took me a while to understand what was going on.

To Reproduce

  1. Question > Sample > Products
  2. Filter CreatedAt > Exclude > "Days of the week" > uncheck Monday - Add filter
  3. Incorrectly shows the wrong day - it is filtering correctly in the query
    image
  4. Edit the filter and select Monday again, now it's not possible the save the filter changes (Update filter)
    image

Expected behavior
While I'm fine with the order of the weekdays, then in a perfect world, it should order depending on Admin > Settings > Localization > First day of the week

Information about your Metabase Installation:
0.43.0-rc1 and master b9cedcc

@flamber flamber added Type:Bug Product defects Priority:P2 Average run of the mill bug .Correctness Querying/Parameters & Variables Filter widgets, field filters, variables etc. .Frontend .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. labels Apr 23, 2022
@flamber flamber changed the title Filtering "Exclude" shows wrong weekend Filtering "Exclude" shows wrong weekday Apr 24, 2022
@nemanjaglumac nemanjaglumac added this to Backlog in Cypress Testing via automation Apr 25, 2022
nemanjaglumac added a commit that referenced this issue Apr 25, 2022
@akiselev
Copy link
Contributor

akiselev commented Apr 25, 2022

@flamber what is the correct behavior for the second part?

If the user selects nothing to exclude, the resulting filter would be ["!=", ["field", ...]] which isn't a valid expression. Update filter in this case should remove the filter altogether in order to keep it valid?

@flamber
Copy link
Contributor Author

flamber commented Apr 25, 2022

@akiselev I would expect to be able to deselect and select something again.
If it should remove the filter or now, perhaps, I'm not sure.

@akiselev
Copy link
Contributor

@flamber Only ["!=", ["field", ...], null] works without removing the filter altogether but I'm not sure about the semantics. Does setting 'temporal-unit' along with != null result in a field is not null?

@flamber
Copy link
Contributor Author

flamber commented Apr 25, 2022

@akiselev I don't know the code, so probably better if you ask one of the BE devs.

@akiselev
Copy link
Contributor

@flamber is extract(day_of_week FROM my_field) IS NOT NULL acceptable semantics here when nothing is excluded? Otherwise I think we'll have to remove the filter

@flamber
Copy link
Contributor Author

flamber commented Apr 25, 2022

@akiselev If it makes more sense to remove the filter, then do that. You should ask someone who has been part of the team working on this feature, since I don't know what the intention was, but perhaps it's somewhere in Notion.

This was referenced Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.Correctness .Frontend Priority:P2 Average run of the mill bug Querying/Parameters & Variables Filter widgets, field filters, variables etc. .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants