SAP / openui5-docs

OpenUI5 Markdown Documentation
https://sap.github.io/openui5-docs/
Creative Commons Attribution 4.0 International
73 stars 58 forks source link

"Dates, Times, Timestamps, and Time Zones": missing sub header for JSONModel ↔ ODataModel #81

Closed boghyon closed 1 year ago

boghyon commented 1 year ago

The content of the new topic "Dates, Times, Timestamps and Time Zones" is really top-notch and I believe it will be very helpful for application developers and customers understanding and learning best practices.

One thing I noticed is that the section "Best Practices" has several subsections for each one of the cases expectedly, except for the case regarding the JSON model ↔ OData model.

Current

### Best Practices

...

JSON models can also be used if the data is stored in the JSON model in the same way as in the corresponding OData model. [...]

Expected

It would be nice if the JSON model ↔ OData model case could also get a heading like the other subsections in within the "Best Practices" section. For example:

### Best Practices

...

#### sap.m.DatePicker with Edm.Date or Edm.DateTime (OK)

...

#### sap.m.TimePicker with Edm.TimeOfDay or Edm.Time (OK)

...

#### sap.m.DateTimePicker with Edm.DateTimeOffset (OK)

...

#### sap.m.DateRangeSelection with Edm.Date or Edm.DateTime (OK)

...

#### <Header for JSONModel ↔ ODataModel> (← missing)

JSON models can also be used if the data is stored in the JSON model in the same way as in the corresponding OData model. [...] ...

Alternatively (See @uhlmannm's reply below)

In my view, explaining how to "transfer" data between JSONModel ↔ ODataModel is questionable since the case doesn't really reflect current best practices but rather encourages anti-patterns such as requesting a large amount of entities via v2.ODataModel#read, storing the response in a JSONModel, then surgically manipulating the data manually instead of working with OData bindings.

The JSONModel ↔ ODataModel case should be removed completely.

uhlmannm commented 1 year ago

Wherever possible the control should directly be bound to the ODataModel. That is not possible, though, in a filterbar. In that case, binding against a JSONModel with the proper type required in OData is the way to go. The values are transported to filter objects and not into the ODataModel properties. However, not every use case can be provided with the ODataModel. For some, like the V4 Tree we are on it. Until we are there, applications will be able to help themselves using a JSONModel. How to transfer the values should be shown here as well.

KlattG commented 1 year ago

I've added a note with a recommendation to bind controls directly to the OData model, and to use JSON models only where directly binding to the OData model does not work. I've also added a separate heading for the JSON model ↔ OData model case. See https://help.sap.com/docs/SAPUI5/96880755e4e64fcd96c12694f430fece/6c9e61dc157a40c19460660ece8368bc.html?locale=en-US&state=DRAFT&version=Internal#best-practices

boghyon commented 1 year ago

Hi @KlattG, hi @uhlmannm,

Thanks for the update! Is there a particular reason why the "Data transfer between an OData and a JSON model" subsection comes first directly under the "Best Practices" section?

Since the "data transfer" is required only in certain cases (e.g. filterbar or V4 tree), and because that subsection is not short, it would be great if that subsection comes last as shown in my initial issue description. What do you think?

Again, thanks for the great documentation!

boghyon commented 1 year ago

Fixed by https://github.com/SAP/openui5-docs/commit/912fd3723e233e4ea29c42d7894152abc08ce396#diff-04e683987fb810d01f5aea2f5d4527dde865b3fd4523631e1e9ee743c79c0857R428 and https://github.com/SAP/openui5-docs/commit/7ba9df89a0ffd3f9eec8ca12929f2d3e8a7ce3f5#diff-04e683987fb810d01f5aea2f5d4527dde865b3fd4523631e1e9ee743c79c0857. Thanks everyone!