EZP-30099: Define preferred user date format #846

pawbuj · 2019-02-19T22:14:06Z

Question	Answer
Tickets	EZP-30099
Bug fix?	no
New feature?	yes
BC breaks?	no
Tests pass?	yes
Doc needed?	yes
License	GPL-2.0

User settings / datetime format.

Checklist:

Coding standards ($ composer fix-cs)
Ready for Code Review

dew326

There are a few BC breaks.

src/bundle/Resources/public/js/scripts/admin.format.date.js

src/bundle/Resources/public/js/scripts/helpers/timezone.helper.js

sunpietro

Besides the comments from @dew326 there are some minor coding standards issues.

src/bundle/Resources/public/js/scripts/admin.user_settings.datetime_format.js

webhdx

I'm not sure how it works for but since you are not mapping siteaccess aware configuration to parameters in \EzSystems\EzPlatformAdminUiBundle\DependencyInjection\Configuration\Parser\UserPreferences::mapConfig. Please check how it's done and add your mapping accordingly.
There are many typos. Please check your code.
I'd store datetime format as a JSON object in User Preferences because combined string makes it too complicated. Just look at \EzSystems\EzPlatformAdminUi\Service\UserPreferences\DateTimeFormat\DateTimeFormatRepository - it requires a lot of strings magic.
Changing \EzSystems\EzPlatformAdminUi\UI\Config\Provider\DateFormat is a BC break.
We are using multiple date formats across AdminUI. Single one is not enough since sometimes we need short format and sometimes full.

src/bundle/Resources/views/user/settings/update.html.twig

src/bundle/DependencyInjection/Configuration/Parser/UserPreferences.php

src/bundle/Resources/config/ezplatform_default_settings.yml

webhdx · 2019-02-20T08:02:09Z

src/bundle/Resources/config/services/ui_config/common.yml

@@ -100,3 +100,7 @@ services:
    EzSystems\EzPlatformAdminUiBundle\Templating\Twig\PathStringExtension: ~

    EzSystems\EzPlatformAdminUiBundle\Templating\Twig\ContentTypeIconExtension: ~
+
+    EzSystems\EzPlatformAdminUiBundle\Templating\Twig\DateTime\DateTimeExtension: ~


It shouldn't be placed in this file. This namespace is reserved for EzSystems\EzPlatformAdminUi\UI\Config stuff. Someone has already put non relevant service definitions here (PathStringExtension and ContentTypeIconExtension).

src/lib/Service/UserPreferences/DateTimeFormat/DateTimeFormatRepository.php

src/lib/UserSetting/Setting/DateTimeFormat.php

adamwojs · 2019-02-20T09:51:49Z

Could you please update PR description with the configuration and ez_datetime usage examples? The high-level overview of the solution will speed up the review in large PRs and usually is initial input for the @DominikaK and @MagdalenaZuba to document feature 😉

lserwatka · 2019-02-22T10:34:48Z

@pawbuj rebase is needed here.

src/bundle/Resources/public/js/scripts/helpers/timezone.helper.js

src/bundle/Resources/public/js/scripts/admin.format.date.js

src/bundle/Resources/public/js/scripts/helpers/timezone.helper.js

src/bundle/Resources/views/link_manager/view.html.twig

dew326 · 2019-03-05T14:46:13Z

src/bundle/Resources/public/js/scripts/admin.user_settings.datetime_format.js

+    const demoContainer = doc.querySelector('#datetime_format_demo');
+
+    const updateDemoDateFormat = () => {
+        let dateFormat = dateDropdown.options[dateDropdown.selectedIndex].value;


dew326 · 2019-03-05T14:46:23Z

src/bundle/Resources/public/js/scripts/admin.user_settings.datetime_format.js

+
+    const updateDemoDateFormat = () => {
+        let dateFormat = dateDropdown.options[dateDropdown.selectedIndex].value;
+        let timeFormat = timeDropdown.options[timeDropdown.selectedIndex].value;


dew326 · 2019-03-05T14:46:33Z

src/bundle/Resources/public/js/scripts/admin.user_settings.datetime_format.js

+    const updateDemoDateFormat = () => {
+        let dateFormat = dateDropdown.options[dateDropdown.selectedIndex].value;
+        let timeFormat = timeDropdown.options[timeDropdown.selectedIndex].value;
+        let datetimeFormat = dateFormat + ' ' + timeFormat;


dew326 · 2019-03-05T14:46:48Z

src/bundle/Resources/public/js/scripts/admin.user_settings.datetime_format.js

+    dateDropdown.addEventListener('change', updateDemoDateFormat, false);
+    timeDropdown.addEventListener('change', updateDemoDateFormat, false);
+    updateDemoDateFormat();
+})(window, document);


new line at the end

dew326 · 2019-03-05T14:47:09Z

src/bundle/Resources/encore/ez.js.config.js

@@ -15,6 +15,7 @@ const layout = [
    path.resolve(__dirname, '../public/js/scripts/button.prevent.default.js'),
    path.resolve(__dirname, '../public/js/scripts/udw/browse.js'),
    path.resolve(__dirname, '../public/js/scripts/admin.user.menu.js'),
+    path.resolve(__dirname, '../public/js/scripts/admin.user_settings.datetime_format.js'),


admin.user.settings.datetime.format.js

src/bundle/Resources/public/js/scripts/admin.user_settings.datetime_format.js

dew326 · 2019-03-05T15:58:09Z

src/lib/UI/Config/Provider/DateFormat.php

        return [
-            'full' => 'M j, Y g:i A',


Is this working? Seems like full is still removed.

Good catch. I forgot to commit this file.

dew326 · 2019-03-05T15:58:41Z

src/bundle/Resources/translations/user_settings.en.xliff

+      <trans-unit id="f99a9bb9741db45f57f2436c535d89609ebc6b0d" resname="settings.short_datetime_format.value.description">
+        <source><![CDATA[Format for displaying Date & Time]]></source>
+        <target state="new"><![CDATA[Format for displaying Date & Time]]></target>
+        <note>key: settings.short_datetime_format.value.description</note>


Shouldn't be part of ezplatform-user?

m-tyrala · 2019-03-05T17:33:26Z

There is an error - both on my local machine, and on Travis test build:

Cannot autowire service "EzSystems\EzPlatformAdminUi\UI\Config\Provider\Dat
eFormat": argument "$dateTimeFormatSerializer" of method "__construct()" ha
s type "EzSystems\EzPlatformUser\UserSetting\Setting\DateTimeFormatSerializ
er" but this class was not found.

Is this a bug, or there are some missing dependencies?

lserwatka · 2019-03-05T18:01:45Z

Looks like changes from ezplatform-user were not taken into account. Restart should help.

lserwatka · 2019-03-05T20:31:51Z

All green 🚀

pawbuj self-assigned this Feb 19, 2019

pawbuj requested review from webhdx, dew326, mikadamczyk, adamwojs, alongosz, andrerom, micszo, Nattfarinn, sunpietro, tischsoic and ViniTou and removed request for webhdx February 19, 2019 22:14

pawbuj added Ready for review Feature labels Feb 19, 2019

This comment has been minimized.

Sign in to view

pawbuj force-pushed the EZP-30099_date_format branch from 2b3a5b3 to 98ebdd5 Compare February 19, 2019 22:25

This comment has been minimized.

Sign in to view

pawbuj force-pushed the EZP-30099_date_format branch from 98ebdd5 to db6e296 Compare February 19, 2019 23:07

dew326 requested changes Feb 20, 2019

View reviewed changes

dew326 requested a review from webhdx February 20, 2019 07:16

sunpietro suggested changes Feb 20, 2019

View reviewed changes

webhdx suggested changes Feb 20, 2019

View reviewed changes

pawbuj requested review from webhdx and dew326 and removed request for andrerom February 20, 2019 09:42

pawbuj force-pushed the EZP-30099_date_format branch from c245e65 to df14354 Compare March 4, 2019 19:56

dew326 requested changes Mar 4, 2019

View reviewed changes

dew326 reviewed Mar 4, 2019

View reviewed changes

src/bundle/Resources/public/js/scripts/admin.format.date.js Outdated Show resolved Hide resolved

dew326 reviewed Mar 5, 2019

View reviewed changes

src/bundle/Resources/public/js/scripts/helpers/timezone.helper.js Outdated Show resolved Hide resolved

webhdx suggested changes Mar 5, 2019

View reviewed changes

src/bundle/Resources/views/link_manager/view.html.twig Show resolved Hide resolved

pawbuj force-pushed the EZP-30099_date_format branch from 257e60e to f509454 Compare March 5, 2019 08:43

dew326 requested changes Mar 5, 2019

View reviewed changes

pawbuj requested review from sunpietro, webhdx and dew326 March 5, 2019 15:23

dew326 requested changes Mar 5, 2019

View reviewed changes

dew326 approved these changes Mar 5, 2019

View reviewed changes

EZP-30099: Define preferred user date format

ef673a1

adamwojs force-pushed the EZP-30099_date_format branch from 524ac36 to ef673a1 Compare March 5, 2019 18:31

lserwatka merged commit b4580ab into master Mar 5, 2019

lserwatka deleted the EZP-30099_date_format branch March 5, 2019 20:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EZP-30099: Define preferred user date format #846

EZP-30099: Define preferred user date format #846

pawbuj commented Feb 19, 2019

This comment has been minimized.

This comment has been minimized.

dew326 left a comment

sunpietro left a comment

webhdx left a comment •

edited

Loading

webhdx Feb 20, 2019

adamwojs commented Feb 20, 2019

lserwatka commented Feb 22, 2019

dew326 Mar 5, 2019

dew326 Mar 5, 2019

dew326 Mar 5, 2019

dew326 Mar 5, 2019

dew326 Mar 5, 2019

dew326 Mar 5, 2019

adamwojs Mar 5, 2019

dew326 Mar 5, 2019

m-tyrala commented Mar 5, 2019

lserwatka commented Mar 5, 2019 •

edited

Loading

lserwatka commented Mar 5, 2019

EZP-30099: Define preferred user date format #846

EZP-30099: Define preferred user date format #846

Conversation

pawbuj commented Feb 19, 2019

Checklist:

This comment has been minimized.

This comment has been minimized.

dew326 left a comment

Choose a reason for hiding this comment

sunpietro left a comment

Choose a reason for hiding this comment

webhdx left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamwojs commented Feb 20, 2019

lserwatka commented Feb 22, 2019

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

m-tyrala commented Mar 5, 2019

lserwatka commented Mar 5, 2019 • edited Loading

lserwatka commented Mar 5, 2019

webhdx left a comment •

edited

Loading

lserwatka commented Mar 5, 2019 •

edited

Loading