Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid date returned from contentResource.getById #13477

Closed
bjarnef opened this issue Nov 25, 2022 · 22 comments · Fixed by #13500
Closed

Invalid date returned from contentResource.getById #13477

bjarnef opened this issue Nov 25, 2022 · 22 comments · Fixed by #13500

Comments

@bjarnef
Copy link
Contributor

bjarnef commented Nov 25, 2022

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

10.2.1

Bug summary

When I use contentResource.getById() I get document incl. tabs and properties.
However for some

image

but the endpoint GetById does return a valid date.
image

Specifics

No response

Steps to reproduce

Request different documents usings contentResource.getById. Depending on the date, some of these are returned as invalid date from angular resource.

In this case date is 11/20/2022 12:00:00 AM which was picked from a date picker and property editor afterwards changed to label, which is shown on the content node. I should also be mentioned that I afterwards changed datatype of the property from date picker to label (datetime), so the nodes which has been saved afterwards may store the value in a different format.

image

Expected result / actual result

No response

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 25, 2022

@nul800sebastiaan @Zeegaan any idea why the date if different than the one returned from the endpoint?

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 25, 2022

For another node I get a totally different year than expected 😅

The date selected from date picker was 12/1/2022 (date only).

image

image

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 25, 2022

I think the issue may be inside umbDataFormatter.formatContentGetData()

formatContentGetData: function (displayModel) {

which is called here:

return $q.when(umbDataFormatter.formatContentGetData(result));

@elit0451
Copy link
Member

elit0451 commented Nov 25, 2022

Hi @bjarnef 👋
Before asking you for more detailed "Steps to reproduce" (perhaps an isolated simple test scenario where the problem occurs), so I can replicate quicker the issue you are seeing, could you tell me what is the DateTime format you are using? As in his comment on another issue, Nikolaj explained that moving away from the default configuration YYYY-MM-DD causes some problems 🤔 Perhaps it is something related

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 25, 2022

Hi @elit0451

It just used the standard datatype instance "Date Picker".

image

Afterwards I changed the property editor to "Label (datetime)" as these data will be imported via a Hangfire job, so using the date picker was mainly to set a data and get a value in the property, but I think it shouldn't matter as the value returned from the endpoint looks correct.

Furthermore no content types or properties vary by culture, but we have these languages. We could delete en-US but I recall Deploy will re-create this anyway each time a new environment is created on Umbraco Cloud.

image

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 25, 2022

I tried changing the property editor back to Date Picker.

image

it seems to make a difference ad the correct date is fetched from contentResource.getById:

image

The other one mentioned before showing "Invalid date".

image

Maybe there's a difference how date format is stored from date(time) picker and in label (datetime) and how the value is returned from API / Angular.

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 25, 2022

For another node I needed to save the node again after change datatype from label (datetime) >date picker as I got "Invalid date". Afterward the date looks correct.

I suspect the issue happens when changing datatype from date picker > label (datetime) and save the node again. I guess it may be storing the date in a different format.

@elit0451
Copy link
Member

Thanks for investigating! 🙌 Can I get you to update the issue description with those last relevant details and perhaps come up with a simple test scenario that we can work with when reproducing/trying to fix the issue? 🙂

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 25, 2022

I tried this as a test.

@inject Umbraco.Cms.Core.Services.IContentService contentService;
@{
    var node = contentService.GetById(1215);

    <pre>@(node?.GetValue("expirationDate"))</pre>
}                                      

First using date picker (default configuration and saving node) which return this:

<pre>01.12.2022 00.00.00</pre>

image

Next a changed property editor to label (datetime) and saved node once again.
Still returning this

<pre>01.12.2022 00.00.00</pre>

and it also seems correct in the property in backoffice.

image

image

In the table the property is formatted directly.

Utilities.forEach(vm.results.items, item => {
    item.editPath = `/content/content/edit/${item.id}`;

    formatDatesToLocal(item, 'L');
});

function formatDatesToLocal(item, format) {

    format = format || "LLL";

    // get current backoffice user and format dates
    userService.getCurrentUser().then(currentUser => {
        item.expirationDate = dateHelper.getLocalDate(item.expirationDate, currentUser.locale, format);
    });
}

in overlay same approach it use, but it fetch document from id and its properties.

contentResource.getById($scope.model.item.id).then(node => {
    console.log("node", node);
    const properties = getContentProperties(node.variants[0].tabs);

    $timeout(() => {
        vm.properties = properties;
        vm.loading = false;
    }, 250);
});


function getContentProperties(tabs) {

    let properties = [];

    Utilities.forEach(tabs, tab => {

        Utilities.forEach(tab.properties, prop => {

            if (!prop.alias.startsWith("_umb_")) {

                let allowed = [
                    "categories",
                    "expirationDate",
                    "duration",
                    "price"
                ];

                if (allowed.includes(prop.alias))
                {
                    if (prop.alias === "categories" && prop.value) {

                        const udis = prop.value ? prop.value.split(',') : [];

                        getCategories(udis).then(data => {

                            let categories = [];

                            Utilities.forEach(data, item => {

                                categories.push({
                                    name: item.name,
                                    id: item.id,
                                    icon: item.icon,
                                    udi: item.udi
                                });

                                prop.value = categories.map(x => x.name).join("<br>");

                            });
                        });
                    }
                    
                    if (prop.alias === "expirationDate") {
                        formatDatesToLocal(prop, 'L');
                    }

                    prop.editor = "Umbraco.Label";
                    prop.view = "readonlyvalue";

                    properties.push(prop);
                }
            }
        });
    });

    return properties;
}

In this case the date returned from contentResource.getById is different from the original selected in date picker, which is otherwise correct in the table data.

For another node saved after using label (datetime) as property editor, it now init date picker with invalid date.
image

The properties in the overlay is listed using the getContentProperties() function in the controller.

<umb-property
        ng-repeat="property in vm.properties track by property.alias"
        property="property">
        <umb-property-editor model="property"></umb-property-editor>
</umb-property>

the lines here basically makes the properties readonly in this dialog/overlay, while (some of) the properties on the content node still can use editable property editor like date picker, textbox etc. (comment these lines to make it use the original property editor set on document type).

prop.editor = "Umbraco.Label";
prop.view = "readonlyvalue";

@elit0451
Copy link
Member

Just what I was after 🤩 Thanks a lot Bjarne 🙌 , I will have a look and get back to you

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 25, 2022

To reproduce this could be a simple dashboard with a controller and fetch any node:

const nodeId = 1234;
contentResource.getById(nodeId)
    .then(node => {
         console.log("node", node);
    });

Then first start with the a property using Date Time (default configuration) and set a date, e.g. December 1st 2022 and check console from code before and compare property data from GetById and console log.

Next change property editor from Date Time to Label (datetime) and re-save the node.

Go back to dashboard and inspect the property data from GetById and console log. I suspect the values would be different this time.

For other dates e.g. 20th in a month it would probably return an "Invalid date" in the console log as it seems to parse the date incorrect, but it seems the date stored in database and returned from server side is correct. It also explains why this didn't show a difference.

@inject Umbraco.Cms.Core.Services.IContentService contentService;
@{
    var node = contentService.GetById(1234);

    <pre>@(node?.GetValue("expirationDate"))</pre>
}

@elit0451
Copy link
Member

elit0451 commented Nov 28, 2022

Hi again @bjarnef
I've tried setting up a test with a dashboard, changed my backoffice language to Danish, add a Danish culture as well (since the date format is different than the American one) but unfortunately, I couldn't replicate the "Invalid date" response from contentResource.getById that you are getting.. 🤷 I always get the same response from both the GetById action and the console.log of calling contentResource.getById.

As a last resort, could you try the simplified scenario with the dashboard that you described in the above comment (#13477 (comment)) and let me know if you still get an "Invalid date", as I suspect it could be happening because of what you are doing with the custom formatDatesToLocal() function that you are calling 🤔

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 28, 2022

Hi @elit0451

Regarding my backoffice user it is isn't default "English (United States) - en-US" language.

I tried to comment the formatDatesToLocal() function, but it didn't seem to be the issue as I also got the error here:

const nodeId = 1234;
contentResource.getById(nodeId)
    .then(node => {
         console.log("node", node);
    });

With the GetById API returned the expected data, but the Angular resource returned "Invalid date" or an incorrect date, see screenshots here: #13477 (comment)

also when I tested this I got the correct date. So I guess server side everything is as exspected.

@inject Umbraco.Cms.Core.Services.IContentService contentService;
@{
    var node = contentService.GetById(1234);

    <pre>@(node?.GetValue("expirationDate"))</pre>
}

But I think Anguar something modify the data returned from GetById until returned in the angular resource.

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 28, 2022

Hi @elit0451

I have made a nice dashboard for you using default starter kit 😁
Just extract this to App_Plugins folder: MyDashboard.zip

On Home Page node I have added this properties using Date Picker with default configuration.
image

image

Next on Home Page node set December 1st 2022 as start date (v11 release date! 🎉🥳)

image

In dashboard it should show this clicking the "Show details" button.

chrome_UjG8Li8JwT.mp4

Start Date and End Date properties are included, but only End Date is formatted.

If I then change End Date property to using "Label (datetime)" instead.

image

then I get this - notice start and end dates hasn't changed, but end date is formatted as 01/20/2012

chrome_HYOjmx93eZ.mp4

The locale is en-US but even before formatting date it log the content node, which property value is 01/20/2012:

image

even though the date stored in database and returned from GetById API method is 12/1/2022 12:00:00 AM:
image

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 28, 2022

Actually I think you are right that formatDatesToLocal() seems to affect the response I am seeing the the developer console, because when I comment that I get this instead.

image

If I modify this in the details controller then end date property in node has the correct date 12/1/2022, but for vm.node the property value is modified as shows 01/20/2012.

function onInit() {
  vm.loading = true;

  contentResource.getById($scope.model.id)
    .then(node => {

    console.log("node", node);
    vm.node = Utilities.copy(node);
    console.log("vm.node", vm.node);

    const properties = getContentProperties(vm.node.variants[0].tabs);

    $timeout(() => {
      vm.properties = properties;
      vm.loading = false;
    }, 250);
  });
}

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 28, 2022

I tried with formatting of date directly using momentjs https://momentjs.com/guides/#/parsing/known-formats/

function formatDatesToLocal(prop, format) {

  format = format || "LLL";

  // get current backoffice user and format dates
  userService.getCurrentUser().then(currentUser => {
    console.log("locale", currentUser.locale);

    prop.value = moment(prop.value, 'YYYY-MM-DD HH:mm:ss').locale(currentUser.locale).format(format);
    //prop.value = dateHelper.getLocalDate(prop.value, currentUser.locale, format);
  });
}

where YYYY-MM-DD HH:mm:ss is same date format it use in dateHelper.getLocalDate(), but it could also use moment.ISO_8601 if the date was in the standard format.

dateVal = moment(date, 'YYYY-MM-DD HH:mm:ss');

But from date(time) picker the format is 2022-12-01 00:00:00 (or at different format in the configuration) in this case just YYYY-MM-DD (but it can include time as well).

I think the property using "Label (datetime)" property editor may return a different formatted value than when using "Date Picker":

image

So parsing this date is no longer in format YYYY-MM-DD HH:mm:ss, but MM/DD/YYYY HH:mm:ss

This seems to work instead.

function formatDatesToLocal(prop, format) {

  format = format || "LLL";

  // get current backoffice user and format dates
  userService.getCurrentUser().then(currentUser => {
    prop.value = moment(prop.value, 'MM/DD/YYYY HH:mm:ss').locale(currentUser.locale).format(format);
    //prop.value = dateHelper.getLocalDate(prop.value, currentUser.locale, format);
  });
}

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 28, 2022

Maybe "Label (datetime)" should have a format property as well?

image

Or maybe the functions in dateHelper should allow specifying a different parsing format than YYYY-MM-DDTHH:mm:ss - either new functions or existing making to last parameter "optional" so it still use YYYY-MM-DDTHH:mm:ss is parameter value is null or undefined?

Something like this:

getLocalDate: function (date, culture, format, dateStringFormat)  {

    dateStringFormat = dateStringFormat || 'YYYY-MM-DD HH:mm:ss';
    
    if (date) {
        var dateVal;
        var serverOffset = Umbraco.Sys.ServerVariables.application.serverTimeOffset;
        var localOffset = new Date().getTimezoneOffset();
        var serverTimeNeedsOffsetting = -serverOffset !== localOffset;
        if (serverTimeNeedsOffsetting) {
            dateVal = this.convertToLocalMomentTime(date, serverOffset);
        } else {
            dateVal = moment(date, dateStringFormat);
        }
        return dateVal.locale(culture).format(format);
    }
}

I wonder it that also we fix other issues, where developers has change Date (Time) Picker to another format than YYYY-MM-DD or YYYY-MM-DD HH:mm:ss?

@elit0451
Copy link
Member

Nice investigation work Bjarne and super awesome set-up test 🤩 (#13477 (comment)). It was a great idea to send the zip file over!

I didn't have time to go through all the details of it today but I can see that you worked/are working on some PRs related to dates? Shall I let you to it and once we have the PRs merged we can close this issue? 😍

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 29, 2022

Hi @elit0451

This PR has been merged #13469 ... but I think we need to new one to specify date string format.

It could be something like this:

getLocalDate: function (date, culture, format, dateStringFormat)  {

    dateStringFormat = dateStringFormat || 'YYYY-MM-DD HH:mm:ss';
    
    if (date) {
        var dateVal;
        var serverOffset = Umbraco.Sys.ServerVariables.application.serverTimeOffset;
        var localOffset = new Date().getTimezoneOffset();
        var serverTimeNeedsOffsetting = -serverOffset !== localOffset;
        if (serverTimeNeedsOffsetting) {
            dateVal = this.convertToLocalMomentTime(date, serverOffset);
        } else {
            dateVal = moment(date, dateStringFormat);
        }
        return dateVal.locale(culture).format(format);
    }
}

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 29, 2022

When backoffice has language "English (United States)" the value of label (datetime) is formatted like this:

1/14/2017 12:00:00 AM

if I change backoffice user language to "Danish (Denmark)" I get this formatted value instead.

14.01.2017 00.00.00

So I would probably need to parse this different based on backoffice user language.
Not sure where this value is formatted in the "label (datetime)" property editor.

@elit0451
Copy link
Member

Thanks for your efforts Bjarne. 💪 I hope this helps to clarify how much easier it is to locate a bug if there is any when we have a simple clean scenario as opposed to custom functions or third parties. I think we can agree that there isn’t any issue in converting from date picker to label(datetime), nor does the angular resource return something wrong. The label changes based on culture (da-culture will cause the dates to behave as Danish), but that is just how it is. The conclusion that you came to in #13477 (comment) is better suited for a discussion about a feature request (being able to set a date time format on the label), so I will go ahead and close this issue so we don’t mix the concepts (bug/feature). 🙂

@bjarnef
Copy link
Contributor Author

bjarnef commented Nov 30, 2022

@elit0451 actually I think it is possible to format the datetime here using the config > filter, it is just not something added the the label property editor configuration:
https://github.com/umbraco/Umbraco-CMS/blob/v11/contrib/src/Umbraco.Web.UI.Client/src/views/propertyeditors/readonlyvalue/readonlyvalue.controller.js#L17-L25

However I am not sure how the default value is formatted based on culture. Often it would just be the raw value, but I think in the case there may be something formatting dates based on culture.
https://github.com/umbraco/Umbraco-CMS/blob/v11/contrib/src/Umbraco.Web.UI.Client/src/views/propertyeditors/readonlyvalue/readonlyvalue.controller.js#L24

If would be great if the Label property actually supported this filter and configuration. The filter could probably be based on the selected value type. https://www.w3schools.com/angular/angular_filters.asp

and in this case format could be YYYY-MM-DD HH:mm:ss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants