Page MenuHomeIn-Portal Phabricator
Differential D346

INP-1746 Correct time parsing in date range filter
ClosedPublic
Actions

Authored by erik on Sep 17 2018, 1:41 PM.

Details

Reviewers
glebs
alex
Commits
rINP16669: Fixes INP-1746 - Correct time parsing in date range filter
Summary

Made changes in dearch by date logic.

Test Plan

Preparation

  1. go to Website & Content → Labels & Phrases section
  2. modify some phrase, if all minutes or days in the "Modified On" column are equal
  3. confirm, that are visible records with different minutes and different days in the "Modified On" column

Date/Time configuration - for current language set:

  1. "Date Format" to Y-m-d
  2. "Time Format" to H:i:s
  3. "Input Date Format" to m/d/Y
  4. "Input Time Format" to g:i:s A

Testing "Modified On" column filter

  1. specify one from date only (no time)
  2. confirm, that records with other dates are filtered out
  3. specify one to date only (no time)
  4. confirm, that records with other dates are filtered out
  5. specify one to date only (with time)
  6. confirm, that records with other dates or minutes are filtered out (ignoring seconds)
  7. specify one from date only (with time)
  8. confirm, that records with other dates or minutes are filtered out (ignoring seconds)
  9. specify both from & to dates with time
  10. confirm, that records with dates between specified datetimes (ignoring seconds) are visible, but other are filtered out
  11. specify both from & to dates with time in output format (without seconds)
  12. confirm, that records with dates between specified datetimes (ignoring seconds) are visible, but other are filtered out
  13. specify both from & to dates with time in input format (with seconds)
  14. confirm, that records with dates between specified datetimes (ignoring seconds) are visible, but other are filtered out
  15. specify word "test" in place of from date and empty to date
  16. confirm, that no records found
  17. specify word "test" in place of to date and empty from date
  18. confirm, that no records found
  19. specify word "test" in place of from date and existing among records to date
  20. confirm, that no records found
  21. specify word "test" in place of to date and existing among records from date
  22. confirm, that no records found
  23. in search filter specify date using format from "Input Date Format" field without time (m/d/Y)
  24. confirm that right search results are shown

Diff Detail

Repository
rINP In-Portal
Branch
/in-portal/branches/5.2.x
Lint
Lint ErrorsExcuse: Full file reformatting is not part of this task.
SeverityLocationCodeMessage
Errorcore/kernel/utility/formatters/date_formatter.php:375PHPCS.E.CodingStandard.Commenting.DocComment.TagValueIndentCodingStandard.Commenting.DocComment.TagValueIndent
Errorcore/kernel/utility/formatters/date_formatter.php:375PHPCS.E.CodingStandard.Commenting.FunctionComment.MissingParamCommentCodingStandard.Commenting.FunctionComment.MissingParamComment
Errorcore/kernel/utility/formatters/date_formatter.php:376PHPCS.E.CodingStandard.Commenting.DocComment.TagValueIndentCodingStandard.Commenting.DocComment.TagValueIndent
Errorcore/kernel/utility/formatters/date_formatter.php:376PHPCS.E.CodingStandard.Commenting.FunctionComment.SpacingAfterParamNameCodingStandard.Commenting.FunctionComment.SpacingAfterParamName
Errorcore/kernel/utility/formatters/date_formatter.php:377PHPCS.E.CodingStandard.Commenting.DocComment.NonParamGroupCodingStandard.Commenting.DocComment.NonParamGroup
Errorcore/kernel/utility/formatters/date_formatter.php:380PHPCS.E.CodingStandard.Commenting.FunctionComment.TypeHintMissingCodingStandard.Commenting.FunctionComment.TypeHintMissing
Errorcore/kernel/utility/formatters/date_formatter.php:393PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.SpaceBeforeCloseBraceCodingStandard.WhiteSpace.ControlStructureSpacing.SpaceBeforeCloseBrace
Errorcore/kernel/utility/formatters/date_formatter.php:393PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.SpacingAfterOpenBraceCodingStandard.WhiteSpace.ControlStructureSpacing.SpacingAfterOpenBrace
Errorcore/kernel/utility/formatters/date_formatter.php:393PHPCS.E.Generic.ControlStructures.InlineControlStructure.NotAllowedGeneric.ControlStructures.InlineControlStructure.NotAllowed
Errorcore/units/helpers/search_helper.php:606PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/units/helpers/search_helper.php:608PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/units/helpers/search_helper.php:609PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/units/helpers/search_helper.php:612PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/units/helpers/search_helper.php:614PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/units/helpers/search_helper.php:617PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/units/helpers/search_helper.php:619PHPCS.E.Generic.WhiteSpace.ScopeIndent.IncorrectExactGeneric.WhiteSpace.ScopeIndent.IncorrectExact
Errorcore/units/helpers/search_helper.php:622PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceAfterCodingStandard.Strings.ConcatenationSpacing.NoSpaceAfter
Errorcore/units/helpers/search_helper.php:622PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceAfterCodingStandard.Strings.ConcatenationSpacing.NoSpaceAfter
Errorcore/units/helpers/search_helper.php:622PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceAfterCodingStandard.Strings.ConcatenationSpacing.NoSpaceAfter
Errorcore/units/helpers/search_helper.php:622PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceAfterCodingStandard.Strings.ConcatenationSpacing.NoSpaceAfter
Errorcore/units/helpers/search_helper.php:622PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceAfterCodingStandard.Strings.ConcatenationSpacing.NoSpaceAfter
Errorcore/units/helpers/search_helper.php:622PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceBeforeCodingStandard.Strings.ConcatenationSpacing.NoSpaceBefore
Errorcore/units/helpers/search_helper.php:622PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceBeforeCodingStandard.Strings.ConcatenationSpacing.NoSpaceBefore
Errorcore/units/helpers/search_helper.php:622PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceBeforeCodingStandard.Strings.ConcatenationSpacing.NoSpaceBefore
Errorcore/units/helpers/search_helper.php:622PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceBeforeCodingStandard.Strings.ConcatenationSpacing.NoSpaceBefore
Unit
No Unit Test Coverage
Build Status
Buildable 886
Build 886: arc lint + arc unit

Event Timeline

erik created this revision.Sep 17 2018, 1:41 PM
erik edited the test plan for this revision. (Show Details)Sep 17 2018, 1:44 PM
glebs accepted this revision.Sep 18 2018, 4:03 AM
glebs accepted this revision.
glebs edited reviewers, added: glebs; removed: alex.
This revision is now accepted and ready to land.Sep 18 2018, 4:04 AM
alex added a project: Restricted Project.Sep 19 2018, 2:09 AM
alex requested changes to this revision.Sep 19 2018, 3:17 AM
alex added a subscriber: alex.
alex added inline comments.
core/units/helpers/search_helper.php
604–606
NOTE: battle against malformed dates entered by user

change into:

if ( is_numeric($from) && $to === null ) {

this way to date entered in incorrect format won't be handled as to date not specified

607
  1. change condition from
$from == strtotime(date('Y-m-d', $from) . ' 00:00:00', $from)

into

date('H:i:s', $from) == '00:00:00'
  1. combine with condition from previous IF statement
  • less code
  • purpose of the code is cleaner
608–609
NOTE: battle against malformed dates entered by user

change into:

if ( $from === null && is_numeric($to) ) {

this way from date entered in incorrect format won't be handled as from date not specified

609–616
  1. change condition from
$to == strtotime(date('Y-m-d', $to) . ' 00:00:00', $to)

into

date('H:i:s', $to) == '00:00:00'
  1. combine with condition from previous IF statement
  • less code
  • purpose of the code is cleaner
615–618
NOTE: battle against malformed dates entered by user

change into:

if ( is_numeric($from) && $to === null || $from === null && is_numeric($to) ) {
    $from = $from === null ? $to : $from;
    $to = $from;
}

this way we won't replace malformed from/to date entered by user with to/from date, that user entered correctly

619–621
NOTE: battle against malformed dates entered by user

change into:

if ( is_numeric($from) && is_numeric($to) ) {

this way only correctly entered dates would be used in filter

619–626
NOTE: battle against malformed dates entered by user

Please add else statement, that would set $filter_value variable value to FALSE (as string).

this way if we've submitted filter with incorrectly formatted date, then we'll see 0 records instead of all records

686–689
NOTE: battle against malformed dates entered by user

return null instead of false

this way recipient would know if user haven't specified any value

719–721

remove else statement, but keep the code inside it

since code inside else statement is always executed it's pointless to wrap return ... statements with it

This revision now requires changes to proceed.Sep 19 2018, 3:17 AM
alex added a comment.EditedSep 19 2018, 3:19 AM

The inline comments with NOTE: battle against malformed dates entered by user line above them are actually UX discovered issues, that weren't part of original task, but without fixing them user would be as confused as now thinking that date entered in incorrect format is actually working as filter, when it's not.

Please also update test plan to reflect handling of incorrectly entered dates.

erik updated this revision to Diff 865.Sep 19 2018, 5:31 AM
erik edited edge metadata.

Made filter out all records when incorrect date entered.

erik edited the test plan for this revision. (Show Details)Sep 19 2018, 5:35 AM
glebs accepted this revision.Sep 20 2018, 6:04 AM
alex requested changes to this revision.Sep 21 2018, 2:39 AM
alex added inline comments.
core/units/helpers/search_helper.php
685–686
  1. extract $value[$type] into temporary variable
  2. apply all manipulations, that were done on $value[$type] to that temporary variable instead

When performing recursive method call the altered $value argument (trimmed & time added, when specified by user) is given and that results in the parsing error.

Test plan fragment to cover this:

  1. for current language set:
    • "Date Format" to Y-m-d
    • "Time Format" to H:i:s
    • "Input Date Format" to m/d/Y
    • "Input Time Format" to g:i:s A
  2. in search filter specify date using format from "Input Date Format" field without time
This revision now requires changes to proceed.Sep 21 2018, 2:39 AM
alex added inline comments.Sep 21 2018, 2:48 AM
core/units/helpers/search_helper.php
697–705

With current test plan this code is never executed on recursive call to this method. I recommend debugging actual code to verify, that it works.

erik updated this revision to Diff 866.Sep 21 2018, 5:54 AM
erik edited edge metadata.

Fixed bug in recursive call parameter.

erik edited the test plan for this revision. (Show Details)Sep 21 2018, 6:00 AM
glebs accepted this revision.Sep 27 2018, 7:11 AM
This revision was not accepted when it landed; it landed in state Needs Review.Feb 9 2021, 2:52 AM
This revision was landed with ongoing or failed builds.
Closed by commit rINP16669: Fixes INP-1746 - Correct time parsing in date range filter (authored by alex, committed by ). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

DiffIDBaseDescriptionCreatedLintUnit
BaseBase
Diff 186316632Sep 17 2018, 1:41 PM
★
★
Diff 286516632Made filter out all records when incorrect date entered.Sep 19 2018, 5:31 AM
★
★
Diff 386616632Fixed bug in recursive call parameter.Sep 21 2018, 5:54 AM
★
★
Diff 497916668rINP16669Feb 9 2021, 2:52 AM
★
★

Diff 866

View Options

core/kernel/utility/formatters/date_formatter.php

Loading...
View Options

core/units/helpers/search_helper.php

Loading...
In-Portal CMS · In-Portal Community Pages · In-Portal Issue Tracker · In-Portal Repository Browser · Quality Assurance · In-Portal CI Server