Closed ascheibal closed 2 years ago
Hi @ascheibal , I think implementing that feature is quite urgent, see for example https://github.com/corona-warn-app/cwa-quick-test-frontend/issues/191#issuecomment-875028131 https://github.com/corona-warn-app/cwa-app-android/issues/3645 https://github.com/corona-warn-app/cwa-app-ios/issues/2975
Although the referenced issues deal with vaccination certificates, they show the kind of trouble that manually entering of standardised names for RATs will introduce. Wallet apps could/should use standardised names for matching different kind of certificates to single persons.
Is there any progress in the implementation of this important feature?
Hi @vaubaehn, this is currently not selected by the stakeholders and was postponed due to other important stuff. Be also aware that most of test certificates that are issued in Germany come from other partners doing the RAT (Quicktest). It's up to their implementation of validating/correcting the input data. Same to vaccination certificates, where the issue is more severe/can cause more trouble, and where the referenced issues point to - we do not implement the issuance service for them. In general, I totally agree with you, but currently cannot say whether this feature will be included in the cwa-quicktest soon.
@ascheibal @ggrund-tsi I'm looking into this right now.
I think this issue needs to be taken care on 2 points:
The solution for the second part would be to update via change events on the name fields. I would like to test my ideas before I commit anything or submit pullrequest.
Is there a way to start a local (working) dev environment?
The webapplication relies on an Identity and Access Management (IAM) Component used to manage the user accounts and to authorize the login. This has currently not been configured to run on local environment.
Maybe there is some kind of dev guide to get a local, rudimentary version of keycloak running for this? I installed a docker version for this and patched the keycloak.json call, but configuration on keycloak seems to be a intense task from my point.
For this issue there is no need to have all api calls present, I only need to get into the interface and open the registration/qr-code forms. Maybe another staging env, like i had on onboarding, would do the job?
@janhoffmann Great you're looking into this!
- input via qr code
- changes via form input should be auto-translated
The solution for the second part would be to update via change events on the name fields.
Not 100% sure, but wouldn't change events also work for QR code input? I assume same events are triggered when fields are filled out by QR module?
I hope they can provide you with a testing environment.
Hi @ascheibal , thanks a lot for your kind reply.
this is currently not selected by the stakeholders and was postponed due to other important stuff.
Full understanding, but I see it's in your mind, and I hope it will get into your discussions soon.
Be also aware that most of test certificates that are issued in Germany come from other partners doing the RAT (Quicktest). It's up to their implementation of validating/correcting the input data.
Yes, I see. There are also different issues that can arise from insufficient 3rd-party-implementations that 'we' came across lately (and I hope you don't mind my pings too much in that context).
But there could be solutions regarding 3rd-party implementations: the transmission of the standardised names could be made optional. If fields are missing, the backend could generate them automatically before further processing. If fields are present, they could be used as transmitted (with or without validation from backend, whether they're correct. I'd prefer with validation...). In a strict approach, an error could be responded when the transmitted standardised names are not validated as correct against the regular names.
My concerns are, that wallet apps (CWA and others) began to implement person certificate containers that hold all kind of certificates (v,t,r). Correct matching of new certificates of a single person to the correct (pre-)existing container becomes crucial. From a pragmatic point of view you may say, it's a 'nice to have' when certificates are matched correctly to corresponding containers. From the 'common user' perspective I expect a high number of complaints, when some certificates are matched correctly, and for others (name mismatch) new containers are created. There is a certain likelihood that containers may become more important in the future, when a sequence of tests is necessary to prove a regular testing effort. Regular tests were already necessary in the 'tourist opening model' of Schleswig-Holstein (i.e., negative tests results every 72 hours), and they're currently introduced in other countries (Thailand for example: https://reisetopia.de/news/ankuenfte-phuket-sandbox-projekt). If something like this would be (re-)introduced later on EU level (or single regions) due to newly increasing incidence rates, it would be good to be able to proof a series of tests via the person certificate containers in the wallet apps. Hence, a fail-safe matching algorithm is quite important. Practically we already see how many problems arise with the vaccination certificates, when pharmacy staff accidentally enter superfluous spaces in one of the certificates, or caps are different between first and second cert.
Matching by standardised names that are automatically generated by input of regular names may not solve all problems but mitigate the most. A good algorithm to create standardised names can sanitize flawed input from manual entry of regular names.
Manually entering standardised names introduces the same problems like any manual input.
Beside this, it will help testing staff, doctors etc. to save some time in data entry.
Same to vaccination certificates, where the issue is more severe/can cause more trouble, and where the referenced issues point to - we do not implement the issuance service for them.
True. Probably it could be a good idea to involve IBM/UBirch into this discussion/process. Their apps seem also to make use of a container model, and from reviews in app stores it looks like they're already using standardised names for matching ('Name mismatch in CWA, but no issue in CovPass').
In general, I totally agree with you, but currently cannot say whether this feature will be included in the cwa-quicktest soon.
Yes, and I assume you're seeing similar reasons like me, why it could be a good idea to implement it. Thus, my long text above is only to structure some arguments if you want to use them to discuss in an upcoming meeting.
Not 100% sure, but wouldn't change events also work for QR code input? I assume same events are triggered when fields are filled out by QR module?
That's why I need to test this locally :-)
@janhoffmann Thanks for your effort! Feel free to ping me when your PR is ready. My coding skills are limited but fair enough to check the algorithm.
@ascheibal In case there is a decision to implement ICAO conform autogeneration rather sooner than later, please check with @janhoffmann who will do it concretely.
Possibly reference implementation by T-Systems? (to be discussed)
Hi @BrittaTSI , could you kindly share the current state of discussion? Is there already a roadmap of implementation? Have a nice afternoon
Hi @vaubaehn - the issue is still in our backlog. There has been not decision yet by the development team on when to implement it.
Hi @kerstin-oppermann-tsi and @BrittaTSI ,
may I kindly ask to re-raise following issues internally (again):
Why: There are cases, when wallet apps (CWA/CovPass) are not able to group a set of certificates to the corresponding person (i.e., grouping vaccination/recovery/test certificates to the right person correctly). The grouping (=matching) works by comparing standardized forename + standardized surname + birthdate. When they match, they're grouped together to a person. Grouping fails for two different reasons: a) the (quantity of) information present in one certificate does not correspond with the (quantity of) information in another certificate. This is the case, when for example one certificate holds the "DR." title but the others don't. Or, one certificate holds a middle name, while the others don't. Or, someone married and changed the surname. b) the information of normal forename and normal surname is the same between certificates, but the transformation (algorithm, manual entry) to standardized names following the ICAO rules was different for some certificates. Until now, it's only a cosmetic problem, when wallet apps are not able to group. But you will have noticed, that soon (ETA mid February) at least CWA but probably also CovPass/CovPassCheck will be able to detect the admission status (i.e., 2G +/B, 2G, ...) of a holder, depending on the stored certificates, and present a corresponding badge (CWA/CovPass) or even validate the status for admission to a venue (i.e., for 2G +/B, CovPassCheck will be able to scan a sequence of certificates). For this application it's a must, that certificates are grouped correctly. DCC ticketing via a validation service will also be affected.
When looking to error source a), TSI can't do much about it - except implementing a visual trigger to the RAT portal (display a text), that staff needs to ensure that the RAT certificate to be issued should exactly correspond to other already stored certificates (vaccination, recovery) in terms of forename, middlename surname, titles and birthdate. For API partners connected to the backend, it would be good to have a mailing to inform them, that it is recommended to implement a staff-client-feedback-loop to ensure that these data match.
When looking to error source b), TSI should now urgently implement the great suggestion of @ascheibal : the ICAO fields in the RAT portal should be pre-filled to avoid typos/wrong transformation to ICAO standard when entering data (still manually) thus failing grouping of certificates. The implementation of ICAO transformation for the RAT portal should exactly produce the results that would result from the algorithm for our Bundespersonalausweis. To have the best quality for 3rd-party-implementations of API partners, the ICAO algorithm that you implement for the RAT portal must be mandatory for them. They should be informed to change/adapt/enhance their ICAO implementation according to the (error free) algorithm that you use. For newly applying API partners, the quality assurance via the "Abnahmetest" needs to be enhanced, that wrong application of ICAO transformation can be detected from your side, before the partner is going into production.
Some sources where these issues had been discussed lately: API partners with flaws in ICAO algorithm: https://github.com/corona-warn-app/cwa-documentation/issues/810 & https://github.com/corona-warn-app/cwa-quick-test-backend/issues/213 Long discussion about problems with grouping certificates: https://github.com/corona-warn-app/cwa-wishlist/issues/705
Inviting some people into this issue to keep track or to facilitate internal discussion: IBM: @oliver-steinbrecher , @molk-ibm SAP: @mlenkeit , @thomasaugsten
For me it's not clear whether the issuance portal for issuing certificates via pharmacies already uses an automatted ICAO transformation. In this case it would be good to align IBM's and TSI's implementations. Same is true for GEMATIK applications/issuance of certificates via doctor's offices.
Please leave us a line, how you will pick up this issue (again). If there is no possibility for implementation due to contracts, please leave a discrete sign so that user community can ask BMG to guarantee a funding for this. Thank you!
FYI: @dsarkar @Ein-Tim @timokoenig @alexcimander
Hi @thinkberg, unfortunately forgot to ping you. Please see https://github.com/corona-warn-app/cwa-quick-test-frontend/issues/185#issuecomment-1021142950
@vaubaehn I totally agree. Correctly typing the standardized name is the most time consuming part in the hole registration process. On the other hand this is a guarantee for disaster, converting strings is a job for code, not for humans.
I want to stress one more point: I was able to use the Portal used by our state vaccination teams, some special chars where stripped right away. In my opinion it's really important to do a reference implementation, share it and make it mandatory for all sides.
The name is standardized using ICAO rules. However, Sometimes name components are taken from different (automated) sources and may include parts not relevant or missing parts of the name. So its not the transliteration, its the data sources that make it hard. And that is nothing easily fixed.
Hi @thinkberg , thanks for your comment. I assume you're relating to Ubirch's parts of ICAO transliteration in the process? The reason why I pinged you is, that there is still a heterogenity of current ICAO implementations from other stakeholders or there is no automated transliteration at all (standardized names are entered manually).
@janhoffmann pinned it to the point in one sentence:
it's really important to do a reference implementation, share it and make it mandatory for all sides.
This is why I (or we) kindly ask you, all stakeholders, to find and agree on one ICAO algorithm reference implementation, and make it mandatory for all connected parties. This includes TSI's RAT portal and their API partners.
It is forseeable that a reasonable portion of the public will run into problems to prove their admission status (i.e., 2G +/B) when two certificates needs to be validated in a sequence and they can't be matched. There are many reasons why that could fail, heterogenity in ICAO transliteration one important among them. But this is an issue that can be addressed well in a joint effort.
Maybe Ubirch's implementation could serve as the reference implementation for all? Please discuss this internally. Thanks a lot.
Edit: what I mean with ICAO transliteration heterogenity: for some API partners ICAO means, spelling everything in capital letters and replace white space with '<', while others implemented the full algorithm.
Here you can find the ICAO Conversion guidelines for the DCC partners https://github.com/corona-warn-app/cwa-quicktest-onboarding/wiki/Anbindung-an-CWA-mit-Verwendung-von-DCCs#icao-conversion
@thomasaugsten thanks for the reference implementation.
I implemented the algorithm and used it on qr-code reading and added a change detection on name.
@kerstin-oppermann-tsi @ggrund-tsi @ascheibal please organize a review and test on my implementation, as I wrote the code without test env.
I really would appreciate if "my" instance would be updated with the change asap :-)
@ggrund-tsi let's have the discussion here:
I implemented the algorithm as stated in the wiki. will add missing chars according to pdf.
@ggrund-tsi i checked SpeakingUrl, i think we don't gain much profit by adding it and add the cost of an external lib we don't control.
The new MR has the cyrillic and arabic translations as defined in the standard.
One thing we all should agree on: The solution here will never be perfect, because the correct (mathematical) solution depends on more information (f.e. the language) and some translations are not 1:1, but 1:n. I believe this is the best we can do, without being the issuing government.
@janhoffmann I hope you're fine with the ping: https://github.com/corona-warn-app/cwa-quick-test-frontend/pull/290#issuecomment-1030037801 ? If we could get a review from Ubirch, that would be the best outcome the implementation can have - not only for the RAT portal itself, but all connected workflows, thus more happy users (because the likelihood of correctly grouped recovery/vaccination & RAT test DCCs is inecreased) and more happy stakeholders (because less user complaints about ungrouped DCCs that bind much resources on SAP's/IBM's side)...
@vaubaehn of cause.
One more thought: Maybe it would be wise to start with "my" proposed changes, even if we could not do a full sync with other stakeholders. The situation right now is, that most of the persons entering those values don't realize the effect and therefore might not care to do it right at all. Adding the autofill will help those persons a lot to gain better results. Of cause we should aim to have all the same implementation, but we should not wait until we have all stakeholders on board to do a implementation here.
To be clear: I'm fine with any solution and it doesn't need to be my MR, but it needs to be done.
@janhoffmann
but it needs to be done
I fully agree.
Of cause we should aim to have all the same implementation, but we should not wait until we have all stakeholders on board to do a implementation here.
The two goals are: getting better data quality into the RAT portal & providing the best possible reference implementation for future use. Logically, for me the reference implementation would be first step, then implementing it into the RAT portal as the second step. Screening/Reviewing your PR shouldn't be much effort, and just by having a fast look to yours, likely there won't be any changes nescessary. Because that's not too complicated, my hope is that Ubirch could do this in a matter of days, when the value of their review is well understood there.
But in any case I'd agree that their review shouldn't be a long lasting blocker. Maybe waiting one week at most, and then move on anyway?
@vaubaehn Did you hear anything about this?
Good morning, @janhoffmann , unfortunately I didn't hear anything until now. Due to acute disease I also can't do much currently, so please go ahead anyways.
@vaubaehn @ggrund-tsi any news on this topic?
As we have illness and vacations in the team, I tried to check the PR with cyrillic and arabic letters, but for me it didn't work. If I insert cyrillic or arabic letters into the name field they will not automatically transformed and inserted into the ICAO fields.For latin letters it works. But as @ggrund-tsi mentioned cyrillic and arabic letters are the important fields that have to fill. So we decided to wait for developers are back.
@kerstin-oppermann-tsi Thanks for your feedback! Great that you had a look into it, and that it's in your backlog. I favor with @janhoffmann that highest priority is to get it implemented in a working version. However, as even the ICAO guideline shows some blurring with regard to a few special cases of transliteration, it would be great to align with Ubirch's algorithm. Do you, Kerstin, have a direct contact to Matthias Jugel from Ubirch to ask whether they can have a look to the here proposed algorithm, either before or after merging? Alternatively, Ubirch could publish their algorithm via their certification-api repository on GitHub, so that we can take care of the alignment.
@vaubaehn no I don't unfortunatly. And as I said we had to wait for our main developers and scoping to look and decide what to do.
@kerstin-oppermann-tsi which version did you check? Is there a way for me to get a running env to test the proposed changes?
Hi @kerstin-oppermann-tsi , thanks for your reply!
And as I said we had to wait for our main developers and scoping to look and decide what to do.
Yes, for the technical aspects, that's the best.
However, I will open an issue at Ubirch's repo, and again try to establish a collaboration on the ICAO algorithm itself, as soon as I find some time. They should either have a look to the algorithm before or after TSI merged the working version of @janhoffmann 's PR to the RAT portal, or they could opernsource their algorithm, so we can adapt in at a later point in time just in case there are some unexpected deviations for some transliteration.
Hi all, I hope TSI devs have recovered and can have a look to @janhoffmann 's PR soon. Independently, I openend an issue at Ubirch's repo: https://github.com/Digitaler-Impfnachweis/certification-apis/issues/232 There are some examples that illustrate why it was good to have one single reference implementation (fuzzy transliteration even suggested in the ICAO guideline) and that Ubirch could provide their solution.
@ggrund-tsi @fOppenheimer - das Issue können wir schließen, richtig?
Feature description
The standardized name fields are neccessary for DCC conformity.
Problem and motivation
documentation: 9303_p3_cons_en.pdf
example implementation in python for substitution: https://github.com/Arg0s1080/mrz/blob/master/mrz/generator/dictionaries/latin_based.py