mageUtils.convertToMomentFormat does not convert lowercase "m" to moment format

[mattijv]

Preconditions (*)

1. Magento 2.4-develop

Steps to reproduce (*)

1. Call mageUtils.convertToMomentFormat (lib/web/mage/utils/misc.js) with a date format that includes a lower case "m". https://github.com/magento/magento2/issues/28740#issuecomment-644656245

Expected result (*)

1. A valid moment.js format is returned.

Actual result (*)

1. Invalid moment.js format is returned.

Usage scenario

We have a date picker element that uses the date picker knockout bindings

<input type="text" data-bind="datepicker: {storage: date, options: config}">

The dateFormat of the datepicker is dd.m.yyyy. When something causes the knockout bindings to update (see module-ui/view/base/web/js/lib/knockout/bindings/datepicker.js update method), a newDate is calculated by converting the value stored in the observable with moment.js. Now, however, as the utilized convertToMomentFormat method does not convert the lowercase "m" in the format, the format string is not valid for moment.js and the calculated newDate is wrong. This leads to the date picker suddenly changing the date it contained.

Interestingly, the convertToMomentFormat method used to convert the lowercase "m" correctly, but that was removed in https://github.com/magento/magento2/commit/972adf482d78f9a2cfd513bbf7a90063773eaad1#diff-b97c64c3eb1ba3418fe705c823349ce2 for no apparent reason.

The problem could be fairly easy to fix by adding the lowercase "m" handling back, but as I don't understand the reason it was changed, I'm hesitant to make a PR so I don't break something else or cause a regression.

---

• [x] Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.

[m2-assistant[bot]]

Hi @mattijv. Thank you for your report. To help us process this issue please make sure that you provided the following information:

• Summary of the issue
• Information on your environment
• Steps to reproduce
• Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, please, review the Magento Contributor Assistant documentation.

Please, add a comment to assign the issue: @magento I am working on this

---

• Join Magento Community Engineering Slack and ask your questions in #github channel.

[mattijv]

Easiest way to reproduce the issue is to open the developer tools javascript console on any Magento site and writing the following

const utils = require('mageUtils');
utils.convertToMomentFormat('dd.m.yyyy');

which should output "DD.m.YYYY" in the console, whereas the expected result would be "DD.M.YYYY".

[m2-assistant[bot]]

Hi @engcom-Delta. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

• [ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).

  Details

  If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.

• [ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.

• [ ] 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

• [ ] 4. Verify that the issue is reproducible on 2.4-develop branch

  Details

  - Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
  - If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
  - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

• [ ] 5. Add label Issue: Confirmed once verification is complete.

• [ ] 6. Make sure that automatic system confirms that report has been added to the backlog.

[magento-engcom-team]

:white_check_mark: Confirmed by @engcom-Delta Thank you for verifying the issue. Based on the provided information internal tickets MC-38361 were created

Issue Available: @engcom-Delta, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

[mattijv]

As far as I can see, this issue remains unsolved in the 2.4-develop branch.

I also very much criticize the Dev.Experience label as the problematic method is obviously working incorrectly, and is causing issues when using Magento UI components with completely valid time formats. Why is the method only formatting the day and year part of the format string, but not the month, even though the format it the same for all of them in moment.js (i.e. capitalized)?

I'd like to hear the reasoning behind the label, @sdzhepa.

The again "code works correctly" is a desirable dev experience...

[sdzhepa]

Hello @mattijv

I set the label Triage: Dev.Experience because Steps to Reproduce does not describe flow for the end-user on Magento UI. This means that need a developer to understand the problem, how it affects Magento functionality, what flows could be broken, what the impact, etc.

Later, this issue was reviewed and discussed at the Public Triage Meeting with developers and was prioritized as P3.

[mattijv]

@sdzhepa In that case, apologies. I was under the assumption that the Dev.Experience label denotes issues only related to the development experience in Magento. Those don't seem to get much action and in my personal experience have been closed without resolution. Obviously incorrect code doesn't seem to be a purely development experience problem. But if the label also covers all issues not (easily) reproducible by user actions then I agree it's appropriate. Thank you for clarifying.

In any case, I'll try to see if it's possible to trigger the issue by some combination of settings and user actions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

[mattijv]

As far as I can see, the format method remains faulty and won't convert the formats correctly. As per moment.js docs, the format for months should be uppercase. (https://momentjs.com/docs/#/displaying/format/)

As such the issue should remain open. I'd be happy to try and find time for creating a pull request if I had any idea why the method was changed to not format the month part correctly in the first place.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 28 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

[mattijv]

The bug still exists in the latest version of the method.

EDIT (to possibly add some more value to this comment): The fix is clear, just revert the single change to the method that was introduced in the commit linked in the original report. Unfortunately the commit provides no context on why the change was made, so I'm hesitant to contribute the fix myself.