Suggestion for displaying Dates in view updates files #52

astridx · 2018-07-20T19:26:06Z

Pull Request for Issue #42.

Summary of Changes

Note the time zone for the creation date
Do not use date of file for showing the modified date in view updated files. Instead of that use date of update.

Testing Instructions

Install an extension
Create an override
Update the extension

Expected result

Creation date and modified date are the same in view updated files in case of the first update.

Modified date is the of the last update, in case of many updates.

Actual result

You have a delay depending on your time zone for creation date.

astridx · 2018-07-20T19:27:07Z

administrator/components/com_templates/Model/TemplateModel.php

@@ -90,7 +90,7 @@ protected function storeFileInfo($path, $name, $template)

 		if ($coreFile = $this->getCoreFile($path . $name, $template->client_id))
 		{
-			$temp->modifiedDate = date("y-m-d h:i:s.u", filemtime($coreFile));
+			$temp->modifiedDate = date("Y-m-d H:i:s", filemtime($coreFile));


This is not necessary here. I only add it for making the date similar to use the format in Joomla\CMS\Date\Date;

Agree with this we can use :)

Now this is fine.

What is missing here is timezone conversion. I'm not sure in what for a timezone the file timestamps are defined but date takes into account the server timezone and we need to store the dates in UTC.

@laoneo Yes, we are storing DateTimeZone of the server in UTC.

@astridx Can you please remove $temp->modifiedDate because we no longer using this right ?

And, remove unset($after[$i]->coreFile); in override plugin line 145.

anuragteapot · 2018-07-21T05:13:58Z

plugins/installer/override/override.php


-			if (empty($pk->modifiedDate))
+			$pk->modifiedDate = '0000-00-00 00:00:00';
+			if (!empty($pk->modifiedDate))


In this case, we never get $pk->modifiedDate empty right? So, modifiedDate is always $createdDate?
In without this PR or previous. When the core file is removed not updated. We did not get modified date because core file is removed right ?
Then, we show the in override list that core file removed.
So, i think just add $pk->modifiedDate = '0000-00-00 00:00:00'; after if statment or condition . Then, its fine ?

astridx · 2018-07-21T06:59:43Z

@Anu1601CS Many Thanks. I had not considered the case that the file was deleted. Now it should be OK, right?

anuragteapot · 2018-07-21T09:50:57Z

plugins/installer/override/override.php

+			$tz = new \DateTimeZone(Factory::getApplication()->get('offset'));
+			$date = new Date('now');
+			$date->setTimezone($tz);
+			$createdDate = $date->toSql(true);

 			if (empty($pk->modifiedDate))


And, here you can check empty of $pk->coreFile right?

…erride_management into dates

astridx · 2018-07-21T11:50:13Z

@Anu1601CS
You're right. I have planned to check if we need the date of the file at all.
I made the changes you suggested. But: The behavior was not as I expected. I have to take a little more time for this. Unfortunately, I do not have much time before Tuesday. If you like, we can merge this branch. So you have the correct views for the documentation.

We can make an issue for deleting unnecessary program code.

anuragteapot · 2018-07-21T13:31:37Z

@astridx If you are not free. So, can I make changes to it ?

ghost · 2018-07-21T14:35:54Z

@Anu1601CS Yes, of course

anuragteapot · 2018-07-21T15:04:16Z

@astridx Done! I hope, I did not make any mistake.

laoneo · 2018-07-22T09:01:28Z

plugins/installer/override/override.php

 			{
-				$pk->modifiedDate = '0000-00-00 00:00:00';
+				$modifiedDate = '0000-00-00 00:00:00';


Here we can take the null date from the database driver to stay on the safe side.

@astridx Can you please take a look?

@laoneo Do you mean this: 2698f34

laoneo · 2018-07-22T09:01:56Z

plugins/installer/override/override.php

+			$tz = new \DateTimeZone(Factory::getApplication()->get('offset'));
+			$date = new Date('now');
+			$date->setTimezone($tz);
+			$createdDate = $date->toSql(true);


This has to be stored in UTC or not?

I think we can store the time in the user time zone.

Why? How do you want to handle then different timezones and DST?

This has to be stored in UTC or not?

If I am right, it is automatically UTC, as UTC is the default time zone in Joomla\CMS\Date\Date (/libraries/src/Date/Date.php).

…erride_management into dates

…rride_management into dates

anuragteapot · 2018-07-25T16:31:03Z

@laoneo Can you please test this PR ?

laoneo · 2018-07-26T07:03:35Z

Why did you merge this one? It is wrong, dates should never be stored in the user timezone.

astridx · 2018-07-26T07:21:18Z

@laoneo How do you like it? Should we store the UTC value in the database and convert it only for display? Or should we even display the UTC value?

anuragteapot · 2018-07-26T07:24:51Z

@astridx I store UTC value in database.

laoneo · 2018-07-26T07:25:15Z

That's what Joomla does, dates are stored in UTC and are displayed in the timezone of the user.

astridx · 2018-07-26T07:27:55Z

@laoneo OK. I will change it. I will do this in a new PR, ok? Or should I revert the old one?

laoneo · 2018-07-26T07:29:20Z

There are two things here to test:

date("y-m-d h:i:s.u", filemtime($coreFile)); this is always in the timezone of the server if I'm not wrong.
$createdDate = $date->toSql(true);has to be $createdDate = $date->toSql();

You can easily test it by changing the Joomla timezone to something like Sidney and the user timezone to something completely different. And when DSt kicks in, we are screwed even more. I wen through all of that with DPCalendar 🙈

* Load correct core files of override files (#2) Start implements loadcorefile() in administrator/components/com_templates/Model/TemplateModel.php * CS (#3) Coding Standards * codingstandards * codingstandards (#4) * Test (#6) Phase 2 (2 part) Mechanism to find correct core file and implementation. * Remove Notice: Only available for html-folder * Remove Warning if core file not found (#11) Thanks. So one part of the issue #12 is done. * Implement the diff view in template manager Implement the diff view in template manager * coding standard (#17) * fix diff (#18) Fix bug in path in case of administrator template override. Fix bug in path in case of administrator template override. * Notification after update and TEST (#16) Find changed files of overridden files and show message. * coding standard (#21) * correction * correction (#26) * Correcthtmlpath (#27) * correction * change oldhtml to newhtml * List of updated override files. (#30) * addcss (#34) * Final Product (#39) Core and Diff view Updated override history list. Quick icon notification plugin. Override control plugin. * save 3 lines :) * New feature show status. (#47) show status in com_template view templates * link * corrected namespace * Button to Switch (#35) * wip add Switcher * wip style switcher * wip style switch make inline and change on off text * wip start with js * wip js * wip delete buttons and make js more robust * wip save to storage * wip delete old code * wip * wip lint * wip css * set default value for switcher * wip make switcher blue * wip * wip * build * correct names * create new functions * fist test code * use onchange * undo installer.min.js * add forgotten new line at the end of css file * correct align * correct compare.es6 - only deleted the toggle part * correct compare.js - only deleted the toggle part * wip * reduce timeout * wrap in funcitons * wip * add use strict to both js-files(compare and toggle) * add the timeout value of 500 again, because 200 are not enought in my case * use css class 'active' for toggle views * add strict * time out for editor * wip * improvments use newActive and switch * correction * width of switcher-spans * correct align * do not use global * wip * removed timeouts * JTEXT to TEXT * forgotton last line * deleted duplicated comments * css fix align * use unnamed functions in es6 * Sql files for fix database (#50) * sql files for database fix * delete space * Suggestion for displaying Dates in view updates files (#52) Correct Dates and do not use date of file any more * Store Date as UTC and show it in server time zone (#57) * modified and created date are created and stored in UTC * convert dates for displaying in model * spar a loop * normalize timezone in view * use language constants for dateformat * JToolbarHelper to ToolbarHelper * CS * namespace * plural * name * clean * text * fx * sin * files * s * Suggestion for language strings (#60) * language strings * correct typo * delete media folder plg_quickicon * add folder plg_quickicon to build/media_src * delete files in media folder * Move media folder - System (#66) * multi * cs * delete files in media folder for joomla toolbar (#67) * Fix button switchers style. (#70) * button * CS * changed uitab.addTab for updated files * Bring back core.js changes. (#69) * core.js * const * fix * form * core * hound * CS * scopr * grid * alpha * cs * lang * only override file * lang * override lang installer * Cs * sub * Update list of core extensions (#71) * Language changes (#76) * update * Update en-GB.com_templates.ini * override JLIB_HTML_PUBLISH_ITEM this is the hover text on the publish icon in the list of files * Change icon (#74) change the icon to use an outline for more consistency * lang * not core (#75) * not core * Update en-GB.plg_installer_override.ini * namespace * cs * Updated files (#82) * Update default_updated_files.php * Update en-GB.com_templates.ini * Update en-GB.com_templates.ini (#81) * Update en-GB.plg_quickicon_overridecheck.ini (#80) * Update en-GB.plg_quickicon_overridecheck.ini (#79) * remove space (#78) * Update en-GB.plg_quickicon_overridecheck.ini * Update en-GB.plg_quickicon_overridecheck.sys.ini * remove hardcoded id * null get function * state * clean * More changes "core" to "original" (#85) * cs * update * plural

astridx added 2 commits July 20, 2018 21:15

dates

00cc4b1

improvment

e479b38

astridx commented Jul 20, 2018

View reviewed changes

astridx mentioned this pull request Jul 20, 2018

Decide what dates we use in view updated files #42

Closed

anuragteapot reviewed Jul 21, 2018

View reviewed changes

correction set modifiedDate to 0000-00-00- 00:00:00 if empty

6958a39

anuragteapot approved these changes Jul 21, 2018

View reviewed changes

anuragteapot reviewed Jul 21, 2018

View reviewed changes

Merge branch 'master' of https://github.com/joomla-projects/gsoc18_ov…

d1c3720

…erride_management into dates

en

b34ea84

laoneo reviewed Jul 22, 2018

View reviewed changes

astridx added 3 commits July 24, 2018 12:42

Merge branch 'master' of https://github.com/joomla-projects/gsoc18_ov…

b55229b

…erride_management into dates

Merge branch 'dates' of https://github.com/joomla-projects/gsoc18_ove…

2a3eed4

…rride_management into dates

get null date

2698f34

astridx merged commit 2c393ff into master Jul 25, 2018

anuragteapot deleted the dates branch July 26, 2018 07:00

astridx mentioned this pull request Jul 26, 2018

Store Date as UTC and show it in server time zone #57

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suggestion for displaying Dates in view updates files #52

Suggestion for displaying Dates in view updates files #52

astridx commented Jul 20, 2018

astridx Jul 20, 2018

anuragteapot Jul 21, 2018

anuragteapot Jul 21, 2018

laoneo Jul 21, 2018

anuragteapot Jul 21, 2018

anuragteapot Jul 21, 2018

anuragteapot Jul 21, 2018

anuragteapot Jul 21, 2018 •

edited

Loading

astridx commented Jul 21, 2018

anuragteapot Jul 21, 2018

astridx commented Jul 21, 2018

anuragteapot commented Jul 21, 2018

ghost commented Jul 21, 2018

anuragteapot commented Jul 21, 2018

laoneo Jul 22, 2018

anuragteapot Jul 24, 2018

astridx Jul 24, 2018

laoneo Jul 22, 2018

anuragteapot Jul 22, 2018

laoneo Jul 22, 2018

anuragteapot Jul 24, 2018

astridx Jul 24, 2018

anuragteapot commented Jul 25, 2018

laoneo commented Jul 26, 2018

astridx commented Jul 26, 2018 •

edited

Loading

anuragteapot commented Jul 26, 2018

laoneo commented Jul 26, 2018

astridx commented Jul 26, 2018 •

edited

Loading

laoneo commented Jul 26, 2018

Suggestion for displaying Dates in view updates files #52

Suggestion for displaying Dates in view updates files #52

Conversation

astridx commented Jul 20, 2018

Summary of Changes

Testing Instructions

Expected result

Actual result

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anuragteapot Jul 21, 2018 • edited Loading

Choose a reason for hiding this comment

astridx commented Jul 21, 2018

Choose a reason for hiding this comment

astridx commented Jul 21, 2018

anuragteapot commented Jul 21, 2018

ghost commented Jul 21, 2018

anuragteapot commented Jul 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anuragteapot commented Jul 25, 2018

laoneo commented Jul 26, 2018

astridx commented Jul 26, 2018 • edited Loading

anuragteapot commented Jul 26, 2018

laoneo commented Jul 26, 2018

astridx commented Jul 26, 2018 • edited Loading

laoneo commented Jul 26, 2018

anuragteapot Jul 21, 2018 •

edited

Loading

astridx commented Jul 26, 2018 •

edited

Loading

astridx commented Jul 26, 2018 •

edited

Loading