End time column update

[MVarshini]

PBENCH-1307

End time column update

Server deletion date is obtained as string in the format YYYY-MM-DD from the response, removed the formatting.

Edit feature is available for Server deletion column so it would be better to maintain the column.

[webbnh]

Looks good...but somehow my "second thought" got lost, so I've recreated it below. 🙁

[webbnh]

Augh! Apparently I failed to commit my "second thought" on my previous review! 😬 My apologies, Varshini!!

My concern here is that the Dashboard should not be relying on the Server to provide the deletion date in the format which the Dashboard requires. That is, we should be separating "representation" from "presentation": the Dashboard should be taking the value provided by the Server (which is a "date" of some sort) and doing its own formatting of it.

So, I suggest creating a new function, e.g.,

export const formatDate = (dateString) => {
  const dateObj = new Date(dateString + "Z");    // Make sure it's in UTC
  const formObj = new Intl.DateTimeFormat("en-US", {dateStyle: "medium"})
  return formObj.format(dateObj);
};

and then calling that here (and at line 376) instead of formatDateTime().

[dbutenhof]

Whoa there: datestring + "Z" is very definitely "relying on the Server date representation". There has to be a better way to force the Date object to UTC.

Secondarily ... server.deletion is a date, not a date-time. Yeah, for display formatting we need to force it to UTC because we don't want to show the wrong localized date; but showing just the normalized date would be better.

It seems Varshini's code avoids this complication by just displaying the server.deletion value directly. Given the inexact nature of the value (we don't guarantee exactly when or if the server will make a deletion pass), I think that's fine for now.

[webbnh]

Whoa there: datestring + "Z" is very definitely "relying on the Server date representation".

I'm not sure that there is any problem with relying on the Server's representation. What I objected to is presenting the raw value from the Server directly to the user -- I wanted that presentation separated from the original representation.

All that the code adding the Z depends on is that the string from the Server lacks a time zone designator; it'll work with basically anything which looks like a date or time -- it just ensures that Javascript interprets it as UTC.

showing just the normalized date would be better.

That's exactly what (I believe) my suggested code does. (That is, even if the dateString includes time information, it will still return only the date portion.)

It seems Varshini's code avoids this complication

Yeah...it reports what the Server said...and, as long as we stick with that string and never hint to Javascript that it's actually a date, we'll be fine...but, it would be better if the Dashboard owned the presentation.

[dbutenhof]

Whoa there: datestring + "Z" is very definitely "relying on the Server date representation". There has to be a better way to force the Date object to UTC.

Secondarily ... server.deletion is a date, not a date-time. Yeah, for display formatting we need to force it to UTC because we don't want to show the wrong localized date; but showing just the normalized date would be better.

It seems Varshini's code avoids this complication by just displaying the server.deletion value directly. Given the inexact nature of the value (we don't guarantee exactly when or if the server will make a deletion pass), I think that's fine for now.