Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve/variant creation form #588

Open
wants to merge 99 commits into
base: release/3.5
Choose a base branch
from

Conversation

loeswerkman
Copy link
Collaborator

@loeswerkman loeswerkman commented Mar 3, 2022

Update the variantCreationForm to enforce mapping and clean HGVS.

This branch builds the checks and the dialogue that make sure the user inserts clean variants into the database. The dialogue serves as a communication tool throughout the whole process: it will inform the user of steps failing or passing, or of any irregularities along the way. This way, we hope to be able to enforce clean HGVS input as much as possible, without it having to cost us user friendliness. All checks are firstly based on getVariantInfo and fixHGVS to check the syntax, followed by a call to VariantValidator for a validation of the positions and to retrieve the mapping.

Closes #562 and is related to #550.

loeswerkman and others added 30 commits October 6, 2021 08:02
If something is off with a given variant, a dialogue will now open to
inform the user of these issues and propose a possible fix.

Also: fixed issue with outdated curly brackets as used to index a
string.
The isHGVS function is created to increase readability when doing
an HGVS check. Before this commit, this was done using
lovd_getVariantInfo($sVariant, false, true); not very readable.
Now this can simply be done using lovd_isHGVS($sVariant).
This large commit stores the outlines of the approach on how to
open the dialogue, perform tests, do the mapping and make sure only
validated variants are passed onto the database.
To sum, this commit:
- simplifies and cleans up parts of the code;
- reduces the amount of output sent to the user, by using images
  instead of sentences to update the user on the status of the steps;
- makes sure the form receives images to communicate status updates
  as well, and blocks all options that should be blocked after some
  input fields have been automatically filled.
We noticed that flush() actually does not fully make sure the output is
 immediately shown to the users. Instead, what happens is this:
When flush() is being called, the buffered output is duly sent to the
 browser. However, the browser (at least as tested on Chrome) only
 starts to execute the sent in code once the script of origin has
 stopped running. This means that flush() cannot be used to dynamically
 update the HTML webpage while processes are active on the background.
Because we realised that the browser does not execute code that
 was sent in before the end of the script using flush(), we now
 use recursive calls instead. This way, we made sure that the user's
 screen will already show the dialogue, even when the mapping is not
 done yet.
If VariantValidator returns false, if we use an unknown build, or if
 syntax was found which VV does not support, we now make sure these
 ARE allowed into the database anyway, since these issues are outside
 of the user's control.
To give more credits to VariantValidator and its crucial role in the
 mapping of the variants, we have added its name to the title of our
 dialogue.
The md5 translation of validated variants will later serve to check
 whether the received input was indeed validated, and can thus be
 sent to the database.
The md5 translation of validated variants will later serve to check
 whether the received input was indeed validated, and can thus be
 sent to the database.
@loeswerkman

This comment was marked as resolved.

src/ajax/check_hgvs_dialogue.php Outdated Show resolved Hide resolved
src/ajax/check_hgvs_dialogue.php Outdated Show resolved Hide resolved
src/class/object_genome_variants.php Outdated Show resolved Hide resolved
Because we can only fully validate human builds, we want to allow users
 to freely edit their non-human variants.
To prevent code injections (XXS attacks) we now make sure that all
 variables are outputted through htmlspecialchars(). To remember that
 particular variables are unsafe, they are inlined (so now you can
 easily see that certain variables come from $_REQUEST).
Also: all variables are urlde- and -encoded to make sure all
 characters will be put in and taken out of the URL correctly.
The original error message would say that the problem lies in the given
 variant type, even when the error array of VV was empty. If this array
 is empty, the problem most likely does not lie with the variant type,
 but is instead an unknown problem.
Copy link
Member

@ifokkema ifokkema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some small stuff left, and I also saw some open comments (e.g., about ACTION).

src/ajax/check_hgvs_dialogue.php Outdated Show resolved Hide resolved
src/ajax/check_hgvs_dialogue.php Show resolved Hide resolved
src/ajax/check_hgvs_dialogue.php Show resolved Hide resolved
src/ajax/check_hgvs_dialogue.php Show resolved Hide resolved
src/ajax/check_hgvs_dialogue.php Show resolved Hide resolved
src/ajax/check_hgvs_dialogue.php Outdated Show resolved Hide resolved
src/ajax/check_hgvs_dialogue.php Outdated Show resolved Hide resolved
@ifokkema
Copy link
Member

Note to self; when going through this code, check how the form handles g.(1234_?)_(?_2345)del; the fix that we provide is also not correct (g.(1234_2345)del, suffix required). Perhaps use part of the code used in the API and the check HGVS interface that also looks at the errors thrown at the fixed variant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement VariantValidator for all genome builds in variant data entry form
2 participants