3liz / qgis-pgmetadata-plugin

QGIS Plugin to manage some metadata from PostgreSQL layer
GNU General Public License v2.0
12 stars 10 forks source link

Translation de #70

Closed effjot closed 3 years ago

effjot commented 3 years ago

I have translated the glossary entries for CC+ODC licenses, roles, categories, frequency and confidentiality, as well as a few link.type entries to German.

I also added the German Data Licence (for German government open data), commit 2cc81da.

We might need to discuss commit 4653876, which isn’t strictly a translation, but a new concept. We often get data from clients that is intended only for use in the projects contracted by them.

Gustry commented 3 years ago

Hi @effjot

Thanks a lot for your contributions.

Can you rebase on top of master ? You would need to edit pg_metadata/install/sql/upgrade/upgrade_to_1.1.0.sql to make the test passing. Otherwise, existing pgmetadata database won't have your translations if they upgrade their database from 1.0.0.

Let me know if you have any questions.

effjot commented 3 years ago

Thanks for your suggestions! I have added some more translations recently, but am a bit busy now. I’ll update the PR on the weekend.

Gustry commented 3 years ago

No worries. We are planning to make a release beginning of september, or at least before the FOSS4G because there is a PgMetadata presentation.

Gustry commented 3 years ago

The migration test is quite tricky. Any issue ?

I would copy the output of the test, but add 'ON CONFLICT DO UPDATE' no ?

effjot commented 3 years ago

Yes, I tried to write the INSERTs and UPDATEs in upgrade_to_1.1.0.sql. I am not quite sure I understand the testing process fully, though. From the info on the plugin homepage I had originally assumed that 90_GLOSSARY.sql is the master file for writing the translations. For testing, I think that a database is built from scratch by applying all upgrade_to_x.x.x.sql files and then comparing a database dump of the glossary table with 90_GLOSSARY.sql. Is this correct?

Therefore, 90_GLOSSARY.sql should be sorted, as I found out with my new additions (IDs 130 and 131 for German National License and “project-related licence”). I added them just below the other licenses, not at the end of the file. Interestingly, the French »Licence ouverte« (and other glossary terms) had also been added in the middle of the file, presumably without testing errors.

What really confuses me now is that in the test, the built database is missing all translated terms. I had tested the INSERT INTO pgmetadata.t_glossary … and UPDATE pgmetadata.glossary … (adapted from your code in upgrade_to_0.4.0.sql) on my own database and they worked as intended. However, the migration test action on Github failed. I have added a cat of the dump from the built database to the test script, as a quick and ugly way to better understand what the glossary built by the script looks like. (I work on this at work, where I use Windows and my toolset is limited; maybe I should switch to developing on Linux, where I have better tools on hand…)

So currently, the new terms (ids 130, 131) are added as expected to the test database. However, the UPDATE for the translations removes the corresponding terms completely.

What does ON CONFLICT DO UPDATE do, and where should I add it?

Gustry commented 3 years ago

Thanks for the explanation.

From the info on the plugin homepage I had originally assumed that 90_GLOSSARY.sql is the master file for writing the translations.

It is true. These files (10, 20, 30...) are used to generate the SQL documentation AND only when creating a new PgMetadata database. They are master.

For testing, I think that a database is built from scratch by applying all upgrade_to_x.x.x.sql files and then comparing a database dump of the glossary table with 90_GLOSSARY.sql. Is this correct?

Yes, Migration are used on existing database, and we want to be sure that the database is strictly the same with someone creating the new database versus someone who has installed a very old version of PgMetadata.

What does ON CONFLICT DO UPDATE do, and where should I add it?

Sorry, I have re-read your migration again, it is alright.

But I agree, this test is very tricky.

Indeed, you have raised a limitation with pg_dump. INSERT INTO are not ordered from the database, but from the harddrive. https://stackoverflow.com/questions/60639348/pg-dump-inserts-to-sql-file-sorted-by-primary-key

I just committed to your branch a hack to order INSERT in the file, which is not perfect because it doesn't sort by PK ...

I will have to find with @mdouchin a better way of testing glossaries. This test works fine for other database objects such as tables, views, functions, triggers.

effjot commented 3 years ago

Thanks for the explanation and fixing the test!

Now, besides the sort order, the translated terms I had UPDATEd in upgrade_to_1.1.0.sql are also no longer a problem. I’m happy, although I don’t understand it ;-)

I had added a few more terms to the glossary before I realised you had fixed the test and 90_GLOSSARY.sql. I’m now ironing out the little bugs that made the test fail again…

Gustry commented 3 years ago

i.e. can this upgrade script execute several times?

No, a migration is executed a single time.

We keep the plugin version in qgis_plugin table to know which migration we need to execute. https://docs.3liz.org/qgis-pgmetadata-plugin/database/tables/qgis_plugin.html

The migration itself is in the BEGIN...COMMIT.

Gustry commented 3 years ago

I had added a few more terms to the glossary before I realised you had fixed the test and 90_GLOSSARY.sql. I’m now ironing out the little bugs that made the test fail again…

Maybe just focus on your migration first, we will see the test at the end.

effjot commented 3 years ago

I think the most important translations are finished now, so maybe you can pull it into master.

Next, I’ll try to add support for license numbers/attributions (my suggestion #71), on a different branch of course.