Datahub (fix): Handle UpdateFrequency not defined

[tkohr]

Follow-up for #679 which forgot to handle the UpdateFrequency if not defined: https://www.geo2france.fr/datahub/dataset/33fb5924-d7ea-4ac5-bf1e-651c0600109e

Also added a commit to the html util functions to better handle undefined inputs. Currently, they are only used for the record.abstract which should always be defined, but on the dev instance this will get rid of the 'undefined' strings displayed in the record-preview

[github-actions]

Affected libs: api-metadata-converter, feature-editor, feature-search, feature-router, feature-map, feature-dataviz, feature-record, api-repository, feature-catalog, feature-auth, ui-search, ui-elements, ui-catalog, util-shared, ui-widgets, ui-inputs,
Affected apps: metadata-converter, metadata-editor, datahub, demo, webcomponents, search, map-viewer, datafeeder,

• 🚀 Build and deploy storybook and demo on GitHub Pages
• 📦 Build and push affected docker images

[fgravin]

LG thanks !

[jahow]

I'm surprised that the update frequency can be null here. I would expect the key to not be there at all if no frequency could be read, @tkohr could you please describe where this null value comes from? is it from the GN index?

[tkohr]

I'm surprised that the update frequency can be null here. I would expect the key to not be there at all if no frequency could be read, @tkohr could you please describe where this null value comes from? is it from the GN index?

Indeed, there is no value at all in the GN response. The null comes from here. We also return null here. Should we replace both with undefined?

[jahow]

updateFrequency is an optional field, so I don't see any reason why we should initialize it with null. I would suggest:

1. Removing this line
2. Returning "unkown" here

IMO these issues come from the fact that we don't use the Typescript strict mode, so we sometimes end up with unexpected null values. This is a recurring problem, and I think we should address it at the root, i.e. not putting null values where they're not expected (easier said than done).

What do you think @tkohr ?

[tkohr]

1. Removing [this](https://github.com/geonetwork/geonetwork-ui/blob/74e14d6e2bf40cee084c6b48e8001a5784b750dc/libs/api/metadata-converter/src/lib/gn4/gn4.metadata.mapper.ts#L25) line

Yes, right, remove it completely.

2. Returning "unkown" [here](https://github.com/geonetwork/geonetwork-ui/blob/74e14d6e2bf40cee084c6b48e8001a5784b750dc/libs/api/metadata-converter/src/lib/iso19139/codelists/update-frequency.mapper.ts#L65)

Why rather unknown than undefined here?

[jahow]

Why rather unknown than undefined here?

Well if we couldn't match the frequency to anything, it is effectively unknown. If there's no frequency at all in the GN4 record then this shouldn't be called. But I'm not 100% sure, so do as you see fit :)

[tkohr]

Yes, right, got it, so I've used unknown now.

[tkohr]

Hum, so not returning null in the mapper broke this test, but I'm struggling to figure out where (in which transformation direction).

[jahow]

Hum, so not returning null in the mapper broke this test, but I'm struggling to figure out where (in which transformation direction).

if it's taking you too far forget it, let's not spend too much time on this

[coveralls]

coverage: 82.796% (-1.1%) from 83.888%
when pulling fa4dc70 on updateFrequency-undefined
into bd29f62 on main.

[tkohr]

Ok, I've kept the null as default value in the update-frequency.mapper, but removed it from initial record in the gn4.metadata.mapper.

[jahow]

thanks!