Made the code more resilient to stale value prop in date filter#17038
Made the code more resilient to stale value prop in date filter#17038lucasbordeau merged 2 commits intomainfrom
Conversation
lucasbordeau
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:7132 This environment will automatically shut down when the PR is closed or after 5 hours. |
Weiko
left a comment
Weiko
left a comment
There was a problem hiding this comment.
Left a comment about a potential refactoring but non blocking
LGMT! @lucasbordeau
| ); | ||
| } | ||
| case 'DATE': { | ||
| if (recordFilter.operand === RecordFilterOperand.IS_RELATIVE) { |
There was a problem hiding this comment.
Seems there is a few duplicated logic between date and datetime in this file, what do you think?
There was a problem hiding this comment.
I'll do it in a follow-up PR.
There was a problem hiding this comment.
@Weiko I did look at what could be refactored and there seems to be enough subtle differences between how plain date and date times are handled for each operator to justify this structure that looks like it could be refactored. I think it's not worth it. Charles mentioned earlier that we could split this big function into smaller functions for each field type though, which would be interesting to do.
|
Hey @lucasbordeau! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
1 similar comment
|
Hey @lucasbordeau! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
Fixes : #17035 There was a bad pattern which risked introduce this bug in `turnRecordFilterIntoRecordGqlOperationFilter` after recent refactor of date logic and date filters, which didn't create any bug during dev time because it wasn't tested with value combinations that existed before the refactor. Code has been re-organized to make it resilient to any wrong value combination. Also fixed a small bug that appeared during QA with `DATE` filter type, which was blocking the save button from disappearing after a save.
2 participants
Fixes : #17035
There was a bad pattern which risked introduce this bug in
turnRecordFilterIntoRecordGqlOperationFilterafter recent refactor of date logic and date filters, which didn't create any bug during dev time because it wasn't tested with value combinations that existed before the refactor.Code has been re-organized to make it resilient to any wrong value combination.
Also fixed a small bug that appeared during QA with
DATEfilter type, which was blocking the save button from disappearing after a save.