Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.56%. Comparing base (cc8754c) to head (ebeceae).
Report is 1 commits behind head on main.

@@           Coverage Diff           @@
##             main   #14275   +/-   ##
=======================================
  Coverage   95.56%   95.56%           
=======================================
  Files        1843     1843           
  Lines       23886    23886           
  Branches     2752     2752           
=======================================
  Hits        22827    22827           
  Misses        802      802           
  Partials      257      257           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I thought this PR was about getting the verbose metadata for the actual data model, which we already have endpoints for in appDevelopmentController, but when looking closer at it I see that it is about the application metadata and not the model metadata. We should first of all adjust the naming of the methods in this PR to not be confused with the appDev GetModelMetadata endpoint.

But also I wonder if this is necessary? We already have endpoints for getting the appmetadatafile which is cached so I guess we do not improve performance much by getting the specific dataType from the api 🤷

Maybe it makes more sense adding a put endpoint in the ApplicationMetadataController?

@standeren

I thought this PR was about getting the verbose metadata for the actual data model, which we already have endpoints for in appDevelopmentController, but when looking closer at it I see that it is about the application metadata and not the model metadata. We should first of all adjust the naming of the methods in this PR to not be confused with the appDev GetModelMetadata endpoint.

Good point, i agree the naming here should be better. I think "metadata" might be a too generic word that doesn't convey enough context. I think "datatype" is used when referring to this data, what do you think about switching out "metadata" with "datatype" for the endpoint naming?

But also I wonder if this is necessary? We already have endpoints for getting the appmetadatafile which is cached so I guess we do not improve performance much by getting the specific dataType from the api 🤷

The idea here is to avoid unnecessary logic on the frontend, and at the same time make the API more adaptable (e.g. if we move to a custom or internal format in the future).
I agree that there is likely no performance gain (more likely a little loss) but i think focusing solely on performance misses the bigger picture.

Maybe it makes more sense adding a put endpoint in the ApplicationMetadataController?

I disagree, because I think ideally the frontend and controller layer should not need to know where the storage places the information, just what the infomation is related to. And in the context of a datamodel it makes sense to me to have endpoints that alter a datamodels information in the controller for datamodels.

@standeren

Good point, i agree the naming here should be better. I think "metadata" might be a too generic word that doesn't convey enough context. I think "datatype" is used when referring to this data, what do you think about switching out "metadata" with "datatype" for the endpoint naming?

Agree.

The idea here is to avoid unnecessary logic on the frontend, and at the same time make the API more adaptable (e.g. if we move to a custom or internal format in the future). I agree that there is likely no performance gain (more likely a little loss) but i think focusing solely on performance misses the bigger picture.

👍

I disagree, because I think ideally the frontend and controller layer should not need to know where the storage places the information, just what the infomation is related to. And in the context of a datamodel it makes sense to me to have endpoints that alter a datamodels information in the controller for datamodels.

👍
But then I think we should emphasize that we are altering the applicationmetadata in the method-names inside SchemaModelService, as mentioned in the other comment 😊