search

noi-techpark / it.bz.opendatahub.databrowser

Explore and navigate through Open Data you need to build your next service.
https://databrowser.opendatahub.com
GNU Affero General Public License v3.0
8 stars 7 forks source link

As Open Data Hub Data Browser user I want that the deprecated fields of a dataset are hidden per default, to avoid using and modifying deprecated fileds. #423

Closed sseppi closed 5 months ago

sseppi commented 10 months ago

As mentioned this morning in the meeting, thanks to a request processed by the Open Data Hub Customer Care team, it emerged the need to hide per default all fields marked as deprecated in the JSON Schema. The data browser user who wants to see also deprecated fileds has to activate it (maybe through a toggle). In that case the deprecated fields has to be clearly marked as deprecated and in addition the comment provided in the JSON Schema should be shown near the fields.

In the screenshot below you can see the example of the 3 "active" fields of the EventShord endpoint that has the label deprecated: true and in the red box there is the comment that should be shown if the deprecated fiels becomes active.

file:///home/stefano/Pictures/Screenshots/Screenshot%20from%202023-09-18%2016-24-11.png

In the screenshot below a I tried to show what should be hidden (fileds in the red box) and I added the toggle to explain better what I have in mind.

Image

Note: It is important that, if the Show deprecated is true, all deprecated fields are clearla shown as deprecated including the comment provided in the JSON Schema.

pkritzinger commented 9 months ago

UI-Design for deprecated attributes:

https://www.figma.com/file/DmuP6Dbv5LzkCNOXrDnWIL/2023?type=design&node-id=1383%3A2538&mode=design&t=CW1pfW1uwovZEbMn-1

pkritzinger commented 9 months ago

@sseppi @gappc we updated the design: https://www.figma.com/file/DmuP6Dbv5LzkCNOXrDnWIL/2023?type=design&node-id=1383%3A2538&mode=design&t=CW1pfW1uwovZEbMn-1

Regarding color: we prefer the viola version, because it looks a bit fresher - but it's up to you to decide.

a-crea commented 7 months ago

Hi @sseppi , I have a doubt about the implementation: where should I check whether a field is deprecated? Regarding your example, https://api.tourism.testingmachine.eu/v1/EventShort/eventshort-11378, I cannot find where to get a field marked as deprecated.

Can you provide me more info?

sseppi commented 7 months ago

Hi @a-crea ,

the information about deprecation and read only of a field is provided in the JSON Schema in Swagger, and in this case the information coming from swagger has the priority over the configuration.

In my point of view, the behaviour should be the following:

  1. If in the JSON Schema an attribute has the label deprecated: true the Data Browser must show it as deprecated
  2. If in the JSON Schema an attribute has the label readOnly: true the Data Browser shouldn't allow the user to change the data anymore.

In case of need, we can discuss it further during the next Sprint Meeting

gappc commented 7 months ago

Hi @a-crea, in order to get the behavior described by @sseppi we need to merge the deprecated and readOnly information from the OpenAPI spec into the config. At the moment I'm in the process of code restructuring to implement mobility support. If it's ok I'll implement the solution to this issue also during that work as it should be a part of config resolution.

Please let me know what you think about it.

a-crea commented 7 months ago

Hi @gappc ok, let me know whenever it's ready, I'll take care of the UI for now and then I'll implement the required logic to show/hide the correct fields :)

RudiThoeni commented 6 months ago

This is a really cool feature ;) good work!

sseppi commented 6 months ago

@MatteoBiasi will show the description of the deprecation provided by the Open API, if there is one. If there is no description the string "This field is deprecated" will be shown.

@pkritzinger will make a design proposal to hide the deprecated fields also from the table view.

@MatteoBiasi will implement the design agreed to deprecate the fields from the table view

pkritzinger commented 6 months ago

@sseppi @gappc @MatteoBiasi

Here's the link for the design to show/ hide deprecated attributes in the table view. https://www.figma.com/file/DmuP6Dbv5LzkCNOXrDnWIL/2023?type=design&node-id=1997%3A3335&mode=design&t=T6pLesaq7CqPCRbo-1

Behavior:

sseppi commented 5 months ago

@MatteoBiasi while testing I noticed that there are still the following issues:

Image

Image

I have seen that in swagger the Name attribute of the RegionInfo and the MunicipalityInfo are marked as deprecated.

Image

CAn you please have a look a this problems and solve it?

gappc commented 5 months ago

@sseppi that could very well be a problem with the parsing of the OpenAPI document. If it's ok for you and @MatteoBiasi @a-crea I'll take a look at it.

MatteoBiasi commented 5 months ago

@gappc sure, if you think that's the problem, then of curse, proceed. If it's not that, then we can check. @sseppi while for the first point, that was added after our initial estimation and we agreed in a meeting that first we were going to merge the integration without that toggle and then we would have added it with the next group of requirements to estimate.

sseppi commented 5 months ago

@gappc sure, if you think that's the problem, then of curse, proceed. If it's not that, then we can check.

@MatteoBiasi I think that the problem too. can you please check and solve it.

@sseppi while for the first point, that was added after our initial estimation and we agreed in a meeting that first we were going to merge the integration without that toggle and then we would have added it with the next group of requirements to estimate.

@MatteoBiasi can you send me per email an effor estimation for this point?

gappc commented 5 months ago

@sseppi @MatteoBiasi @a-crea I think I found the reason for the deprecation mismatch - we use different fields to render the data.

Here are the results for the Region (same applies to Municipalities):

Although the implementation works as intended, the result is most probably not what the user expects. The question is, how to solve this problem.

A simple solution would be to use the LocationInfo.RegionInfo.Id field and InputReferenceCell component also for the table view, but since we need to make an API call to get the region name for each row in the table, this simple solution seems not feasible.

We could probably implement some caching strategy to improve the performance (e.g. load all Locations on first occurence and reuse that information later on), but we need to think carefully about possible problems, e.g. stale caches.

Another problem that came to my mind is, that by showing the referenced data (in this case the name), we no longer show only the data contained in the record itself, but augment it with referenced data. This contradicts the idea to show only the data from the record itself.

MatteoBiasi commented 5 months ago

@gappc this is a tough one. I think loading all locations is not optimal also for a performance matter. At this point, I would rather try to integrate it directly into the API of the table's calendar. Honestly, I find it difficult to see other alternatives. We could implement a local-level cache so that if a user sees a deprecated field on the table view, we cache it and use this information to display it in the Edit View as well. However, this wouldn't work if someone lands directly on the Edit View through a link. I'll hand it over to @sseppi, who might come up with some additional ideas.

MatteoBiasi commented 5 months ago

@sseppi any thoughts about this?

sseppi commented 5 months ago

@MatteoBiasi @gappc thank you for analyzing the problem, let discuss this issue during the next sprint meeting.