Unexpected output: Affected paths: relative - resubmit patch.
⚙ D229 INP-1565 Add support for "relative" date format
Page MenuHomeIn-Portal Phabricator

INP-1565 Add support for "relative" date format
Needs RevisionPublic

Authored by alex on Jun 13 2016, 9:03 AM.

Details

Reviewers
erik
Test Plan

Preparation

  • in IDE
    1. open core/admin_templates/users/users_edit.tpl file for editing
    2. add these lines after <inp2:m_RenderElement name="combined_header" ... line:
<inp2:u_Field name="CreatedOn" format="relative"/><br/>
<inp2:u_Field name="CreatedOn" format="relative:1"/><br/>
<inp2:u_Field name="CreatedOn" format="relative:2"/><br/>
<inp2:u_Field name="CreatedOn" format="relative:3"/><br/>
<inp2:u_Field name="CreatedOn" format="relative:4"/><br/>
<inp2:u_Field name="CreatedOn" format="relative:5"/><br/>
<inp2:u_Field name="CreatedOn" format="relative:6"/><br/>
<inp2:u_Field name="CreatedOn" format="relative:7"/><br/>
---<br/>
<inp2:u_RelatedCreatedOn level="1"/><br/>
<inp2:u_RelatedCreatedOn level="2"/><br/>
<inp2:u_RelatedCreatedOn level="3"/><br/>
<inp2:u_RelatedCreatedOn level="4"/><br/>
<inp2:u_RelatedCreatedOn level="5"/><br/>
<inp2:u_RelatedCreatedOn level="6"/><br/>
<inp2:u_RelatedCreatedOn level="7"/><br/>
===<br/>
<inp2:u_ShortHumanTimeTests/>
  1. save changes
  2. open core/units/users/users_tag_processor.php file for editing
  3. add this method/tag in there:
protected function RelatedCreatedOn(array $params)
{
    /** @var kDBItem $object */
    $object = $this->getObject($params);

    /** @var DateHelper $date_helper */
    $date_helper = $this->Application->recallObject('DateHelper');

    return $date_helper->getHumanTime(abs($object->GetDBField('CreatedOn') - TIMENOW), $params['level']);
}

protected function ShortHumanTimeTests($params)
{
    /** @var DateHelper $date_helper */
    $date_helper = $this->Application->recallObject('DateHelper');

    $tests = array(
        $date_helper->getShortHumanTime(TIMENOW - strtotime('-1 year')),
        $date_helper->getShortHumanTime(TIMENOW - strtotime('-1 year -2 months')),
        $date_helper->getShortHumanTime(TIMENOW - strtotime('-1 year -2 months -3 days')),
        $date_helper->getShortHumanTime(TIMENOW - strtotime('-1 year -2 months -3 days -4 hours')),
        $date_helper->getShortHumanTime(TIMENOW - strtotime('-1 year -2 months -3 days -4 hours -5 minutes')),
        $date_helper->getShortHumanTime(TIMENOW - strtotime('-1 year -2 months -3 days -4 hours -5 minutes -6 seconds')),
    );

    return implode('<br/>', $tests) . '<br/>';
}
  1. save changes
  • in CLI:
    • run the ./in-portal classmap:rebuild command in the root folder of a project (for new PHP classes to be recognized)
  • in Admin Console
    1. go to ConfigurationWebsiteRegional section
    2. import core/install/english.lang language pack (to add new phrases)

Testing

  1. Login to Admin console
  2. Go to User ManagementUsers
  3. Open any user for editing
  4. change the Created On field to be 2 years and some days in the past
  5. Save changes
  6. Open the same user for editing
  7. Confirm, that below the blue bar you'll see this (do the math yourself to confirm, that shown values are correct):
2 years ago
2 years ago
2 years, 7 months ago
2 years, 7 months, 3 weeks ago
2 years, 7 months, 3 weeks, 1 day ago
2 years, 7 months, 3 weeks, 1 day, 21 hours ago
2 years, 7 months, 3 weeks, 1 day, 21 hours, 17 minutes ago
2 years, 7 months, 3 weeks, 1 day, 21 hours, 17 minutes, and 36 seconds ago
---
2 years
2 years
2 years, 7 months
2 years, 7 months, 3 weeks
2 years, 7 months, 3 weeks, 1 day
2 years, 7 months, 3 weeks, 1 day, 21 hours
2 years, 7 months, 3 weeks, 1 day, 21 hours, 17 minutes
2 years, 7 months, 3 weeks, 1 day, 21 hours, 17 minutes, and 36 seconds
===
1y
1y 2m 2d
1y 2m 5d
1y 2m 5d 4h
1y 2m 5d 4h 5mi
1y 2m 5d 4h 5mi 6s
  1. change the Created On field to be 2 years and some days in the future
  2. Save changes
  3. Open the same user for editing
  4. Confirm, that below the blue bar you'll see this (do the math yourself to confirm, that shown values are correct):
since 1 year
since 1 year
since 1 year, 8 months
since 1 year, 8 months, 1 week
since 1 year, 8 months, 1 week, 3 days
since 1 year, 8 months, 1 week, 3 days, 2 hours
since 1 year, 8 months, 1 week, 3 days, 2 hours, 38 minutes
since 1 year, 8 months, 1 week, 3 days, 2 hours, 38 minutes, and 49 seconds
---
1 year
1 year
1 year, 8 months
1 year, 8 months, 1 week
1 year, 8 months, 1 week, 3 days
1 year, 8 months, 1 week, 3 days, 2 hours
1 year, 8 months, 1 week, 3 days, 2 hours, 38 minutes
1 year, 8 months, 1 week, 3 days, 2 hours, 38 minutes, and 49 seconds
===
1y
1y 2m 2d
1y 2m 5d
1y 2m 5d 4h
1y 2m 5d 4h 5mi
1y 2m 5d 4h 5mi 6s
  1. in the output after === logic is like this: the number at each position is larger by 1 than in the previous position (month is larger by 1 than year, the day is larger by 1 than month and so on), but the number of days won't be 2d as expected, because day count in month differs

Diff Detail

Repository
rINP In-Portal
Branch
/in-portal/branches/5.3.x
Lint
Lint ErrorsExcuse: Errors not related to changed code
SeverityLocationCodeMessage
Errorcore/kernel/utility/formatters/date_formatter.php:281PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.SpaceBeforeCloseBraceCodingStandard.WhiteSpace.ControlStructureSpacing.SpaceBeforeCloseBrace
Errorcore/kernel/utility/formatters/date_formatter.php:281PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.SpacingAfterOpenBraceCodingStandard.WhiteSpace.ControlStructureSpacing.SpacingAfterOpenBrace
Errorcore/kernel/utility/formatters/date_formatter.php:315PHPCS.E.CodingStandard.Commenting.FunctionComment.MissingCodingStandard.Commenting.FunctionComment.Missing
Errorcore/kernel/utility/formatters/date_formatter.php:315PHPCS.E.CodingStandard.NamingConventions.ValidFunctionName.NotCamelCapsCodingStandard.NamingConventions.ValidFunctionName.NotCamelCaps
Errorcore/kernel/utility/formatters/date_formatter.php:315PHPCS.E.Squiz.Scope.MethodScope.MissingSquiz.Scope.MethodScope.Missing
Unit
No Unit Test Coverage
Build Status
Buildable 397
Build 397: arc lint + arc unit

Event Timeline

glebs updated this revision to Diff 536.Jun 13 2016, 9:03 AM
glebs retitled this revision from to INP-1565 Add support for "relative" date format.
glebs updated this object.
glebs edited the test plan for this revision. (Show Details)
glebs added 1 JIRA issue(s): INP-1565.
glebs edited the test plan for this revision. (Show Details)Jun 13 2016, 9:04 AM
glebs edited edge metadata.
glebs edited the test plan for this revision. (Show Details)
alex requested changes to this revision.EditedJun 14 2016, 3:48 AM
alex edited edge metadata.

Please:

  1. make changes specified below to transform code into more reusable state
  2. test plan needs to be updated to cover not only indirect DateHelper::getHumanTime method usage, but also it's direct usage
core/kernel/utility/formatters/date_formatter.php
303
  1. add DateHelper class
  2. add kDateFormatter::dateHelper protected property
  3. in kDateFormatter class constructor do recallObject('DateHelper') and store it in above added property
  4. code of this method, that gets removed below becomes part of DateHelper::getHumanTime($seconds, $detalization_level) method
309–335

becomes:

$res = $this->dateHelper->getHumanTime(abs($from - TIMENOW), detalization_level);
339

becomes:

return $res . ' ' . $suffix;
This revision now requires changes to proceed.Jun 14 2016, 3:48 AM
glebs updated this revision to Diff 537.Jun 14 2016, 4:46 AM
glebs edited edge metadata.

Requested changes

alex requested changes to this revision.Jun 14 2016, 5:10 AM
alex edited edge metadata.

Also test plan doesn't reflect addition of new method.

core/units/helpers/date_helper.php
1
  1. missing licensing file comment
  2. missing svn:eol and svn:keywords properties (see how it's done for other PHP files)
7

Please change to:

Returns human representation of given seconds (e.g. `12 days, and 5 hours`).
9

Please change to Date/time interval in seconds. to Seconds.

This revision now requires changes to proceed.Jun 14 2016, 5:10 AM
alex added inline comments.Jun 14 2016, 5:16 AM
core/kernel/utility/formatters/date_formatter.php
43

Actually please use lazy loading like so:

  1. remove this line
  2. on top of RelativeFormat method place following code:
if ( !isset($this->dateHelper) ) {
    $this->dateHelper = $this->Application->recallObject('DateHelper');
}

This would also solve recursion issue, when rebuilding classmap.

glebs updated this revision to Diff 538.Jun 15 2016, 2:33 AM
glebs edited edge metadata.

Requested changes

alex requested changes to this revision.Jun 15 2016, 3:02 AM
alex edited edge metadata.

And test plan still doesn't cover direct method usage of DateHelper class.

core/units/helpers/date_helper.php
3–14
  1. the formatting of File Comment should be broken (as with any other licensing comment in the project)
  2. change year from 2011 to 2016
  3. not seeing SVN properties being added, are they added?

  1. if all File Comments are broken in same way we can easily find & replace them later
  2. current year must be used in the license
70

Please add following comment above this line:

// Using "DateInterval" class directly will lose 61+ second.
71

Please rename $diff_date into $futher_date.


Variable name must represent content inside that variable, not the usage of that variable in the code.

72–73

Please replace with following (it's shorter and hopefully works the same):

$now->setTimestamp(0);
$diff_date->setTimestamp($seconds);
77

Please inline this into return statement below.

This revision now requires changes to proceed.Jun 15 2016, 3:02 AM
glebs edited the test plan for this revision. (Show Details)Jun 17 2016, 1:51 AM
glebs edited edge metadata.
glebs updated this revision to Diff 540.Jun 17 2016, 1:54 AM
glebs edited edge metadata.

Requested changes

alex requested changes to this revision.Nov 20 2016, 1:06 PM
alex edited the test plan for this revision. (Show Details)
alex edited edge metadata.
alex edited edge metadata.
alex added inline comments.
core/kernel/utility/formatters/date_formatter.php
275

Not sure why, but when I apply the patch this fragment is added into Parse method instead of Format method. Probably will be fixed when you resend the patch.

303

Please:

  1. rename method from RelativeFormat to relativeFormat
  2. change detalization_level into level_of_detail (in all files from this patch including DocBlocks)

  1. CS: method in a class naming
  2. PhpStorm highlights this as an error
core/units/helpers/date_helper.php
72

My apologies, but $futher_date turned out to be grammatically incorrect word. Please rename it into $further_date now.

This revision now requires changes to proceed.Nov 20 2016, 1:06 PM
glebs updated this revision to Diff 622.Nov 22 2016, 5:14 AM
glebs edited edge metadata.

Requested fixes

alex requested changes to this revision.EditedNov 22 2016, 5:53 AM
alex edited edge metadata.
In D229#4319, @alex wrote:

And test plan still doesn't cover direct method usage of DateHelper class.

Not addressed.

Also instead of changing unit config it would more realistic to ask to add <inp2:u_Field name="CreatedOn" format="relative"/> tag at the beginning of user editing template. That would make test plan easier to use.

This revision now requires changes to proceed.Nov 22 2016, 5:53 AM
glebs edited the test plan for this revision. (Show Details)Nov 24 2016, 11:08 AM
glebs edited edge metadata.
glebs requested a review of this revision.Nov 24 2016, 11:11 AM
glebs edited edge metadata.
alex updated this revision to Diff 930.Apr 22 2020, 3:49 AM
alex edited the test plan for this revision. (Show Details)
  1. moved handling of "relative" format from "kDateFormatter::Parse" method (where it wasn't working obviously) to the "kDateFormatter::Format" method
  2. restored "DateHelper" class, that was removed in 622 diff by mistake
alex edited the test plan for this revision. (Show Details)Jul 22 2022, 2:17 AM
alex edited the test plan for this revision. (Show Details)Jul 22 2022, 6:44 AM
alex edited reviewers, added: erik; removed: alex.
alex commandeered this revision.Jul 22 2022, 6:47 AM
alex added a reviewer: glebs.
alex removed a reviewer: glebs.
erik requested changes to this revision.Jul 27 2022, 1:43 PM

Math is incorrect.

On tests I set CreatedOn date approximately 2 years and 6 days ago. Result of <inp2:u_Field name="CreatedOn" format="relative:7"/><br/> was "2 years, 2 months, 1 week, 1 day, 2 minutes, and 29 seconds ago"

  1. >2 month difference, because there are approhimately 31M seconds in the year, not 29M (as in DateHelper::getHumanTime)
  2. used approach is doubtful, because seconds in a year, seconds in a month - that are not constant values.
  3. I suggest

3.1) have more precise average seconds for year and month
3.2) use logic like $date_approach = strtotime("-n years", $prev_date_approach), to have more precise calculation, where n may be estimated by current logic, and, if $date_approach suddenly becomes out of desired limit - use correction like $date_approach = strtotime("+1 year", $date_approach)

This revision now requires changes to proceed.Jul 27 2022, 1:43 PM