PB-1580: Extend dataset with title and description #80
Conversation
msom
left a comment
msom
left a comment
There was a problem hiding this comment.
As discussed:
- actually test the import of the new fields in bod_sync
- update the link in the PR template diagram (https://swissgeoplatform.atlassian.net/wiki/spaces/PB/pages/16155637/Business+Entity+Model)
asteiner-swisstopo
left a comment
asteiner-swisstopo
left a comment
There was a problem hiding this comment.
Looks good to me, only some maintainability-related suggestions.
| if not bod_meta.bezeichnung_de: | ||
| bod_meta.bezeichnung_de = "#Missing" | ||
| missing = True | ||
| if not bod_meta.bezeichnung_fr: | ||
| bod_meta.bezeichnung_fr = "#Missing" | ||
| missing = True | ||
| if not bod_meta.bezeichnung_en: | ||
| bod_meta.bezeichnung_en = "#Missing" | ||
| missing = True | ||
| if not bod_meta.abstract_de: | ||
| bod_meta.abstract_de = "#Missing" | ||
| missing = True | ||
| if not bod_meta.abstract_fr: | ||
| bod_meta.abstract_fr = "#Missing" | ||
| missing = True | ||
| if not bod_meta.abstract_en: | ||
| bod_meta.abstract_en = "#Missing" | ||
| missing = True |
There was a problem hiding this comment.
Maybe easier to read as
required_fields = [
bod_meta.bezeichnung_de,
bod_meta.bezeichnung_fr,
bod_meta.bezeichnung_en,
bod_meta.abstract_de,
bod_meta.abstract_fr,
bod_meta.abstract_en,
]
for field in required_fields:
if not field:
field = "#Missing"
missing = TrueWhat do you think?
There was a problem hiding this comment.
I tried this approach, but I think I need to use something like this to update the field:
required_fields = [
"bezeichnung_de",
"bezeichnung_fr",
"bezeichnung_en",
"abstract_de",
"abstract_fr",
"abstract_en",
]
for field in required_fields:
if not getattr(bod_meta, field):
setattr(bod_meta, field, "#Missing")
But then I get an issue with the mypy type checking. So I left it as is.
| bezeichnung_de="some_val", | ||
| bezeichnung_fr="some_val", | ||
| bezeichnung_it="some_val", | ||
| bezeichnung_rm="some_val", | ||
| bezeichnung_en="some_val", | ||
| abstract_de="some_val", | ||
| abstract_fr="some_val", | ||
| abstract_it="some_val", | ||
| abstract_rm="some_val", | ||
| abstract_en="some_val" |
There was a problem hiding this comment.
Are these values compared anywhere in the test? If yes, setting them all to the same value risks not detecting if one is set to the value of another accidentally.
There was a problem hiding this comment.
Thanks. I extended the test cases and changed the values. Actually had a bug in the code..
| self.print(f"Dataset '{bod_dataset.id_dataset}' is missing geocat entry") | ||
| continue | ||
|
|
||
| # REMOVE THIS BLOCK ONCE DATA HAS BEEN FIXED |
There was a problem hiding this comment.
I wasn't sure what is meant here before reading your comment in PB-1580. Maybe be more specific like "Remove this block once all datasets have data in the required fields".
There was a problem hiding this comment.
Removed the comment and slightly changed the behaviour after offline discussions to use fallback language for english and not update update the data.
The link is already updated, see #81. My bad, only just seen your comment now. |
Add title and description in 5 languages to dataset model and migrate database. Add fields to api responses. Extend bod_sync command to also migrate title and description from the geocat_publish table. Update tests.
benschs
force-pushed
the
feat-PB-1580-extend-dataset-meta-info
branch
from
fcbe73d to
896b6d9
Compare
Add title and description in 5 languages to dataset model. As with
Attributionthe german, french and english translations are required. Also added fields to api responses.Extend
bod_synccommand to also migrate title and description from the geocat_publish table. If a required lagnauge is missing, set value to#Missing.