Fix object radio #581
Fix object radio #581
Conversation
to test DA base64 encodings, in SuffolkLITLab/ALKiln#581
BryceStevenWilley
force-pushed
the
fix_object_radio
branch
from
7e289f5 to
31d39ee
Compare
|
TODO:
|
BryceStevenWilley
force-pushed
the
fix_object_radio
branch
from
31d39ee to
f81a49f
Compare
|
Some of the fixtures are failing. A bit braindead to dig into why right now, sorry. Fixtures are sadly fragile. I hesitated to use them in the first place. It seemed like a shame not to unit test them when the output was a string, though. We might discuss removing them. |
|
I am aware and know why, see f81a49f.
Was also having issues with chai not actually printing the diff at all (it just says the assert failed and doesn't print anything else), which was pretty annoying. In general I don't have the bandwidth to work on this for a bit. This was supposed to be a smaller commit (fixing just object radio buttons), but has ballooned in scope to try to handle all of the base64 fields, as well as handling object multiselects and various other fields that I don't have the experience with. |
to test DA base64 encodings, in SuffolkLITLab/ALKiln#581
BryceStevenWilley
force-pushed
the
fix_object_radio
branch
from
f81a49f to
4c7ecf5
Compare
|
Since we mentioned (offline) that we should still include some of the changes from #604 and this PR in the stripped down PR, it's just easier to take out the code trying to handle multiples. That code is now in the I just discovered that I can't run puppeteer in WSL, so unfortunately I can't get things testing and running fully. I anticipate that i'll have to comment out all lines in |
The values of object radio buttons are variable names encoded as base64. Convert those values into the human readabale versions of themselves if they can be converted.
* fromBase64 would accept invalid base64 strings, and just convert it to what it wanted to. This does a rough check to make sure that the value contains only valid base64 characters before trying to convert it. TODO: there could be more work done here to only add values that once decoded, end up as ascii characters (since we're working in python), but that can be future work. * Added more docstrings to the toBase64 and fromBase64 functions * Made unit tests pass by removing bad base64 conversions from the fixtures * switched one of the test argument orders, since the expected and actual values are backwards. TODO: should change the other as well
Tested some unittests that have object multiselect * separated out `fromBase64` and `toBase64`, which were scope only, and not async * separated out nestedDecode from getPossibleVarNames, where we can also use it in getPossibleValues as well * check for `multiple=""` in selects, meaning we can select multiple items and should treat them like check boxes
unit tests are broken because I added `multiple` as an attribute to the fields, so now the deep equals are wrong, features are broken because we can't use multipleselects, object or not. The entire framework of passing info to the funnel needs to change so we can associate the decoded values from story tables to the encoded values in the page.
BryceStevenWilley
force-pushed
the
fix_object_radio
branch
from
4c7ecf5 to
3457d29
Compare
|
I've got tests working on this, with the changes in SuffolkLITLab/docassemble-ALKilnSetup#197. Should be ready for a last review, @plocket |
There was a problem hiding this comment.
The code looks good! Great changes! Some comments on clarity and some typo corrections, but this is probably good to go as far as functionality is concerned. There is one comment I feel is more important to add - the note about capturing da's encoding patterns.
This is a complex constellation of concepts to tackle. Kudos.
Co-authored-by: plocket <52798256+plocket@users.noreply.github.com>
|
I think all of the comments were addressed and confirmed to be addressed or, (in the instance of #616) made into issues. Going to merge now! |
The values of object radio buttons are variable names encoded as base64.
Convert those values into the human readable versions of themselves if they can
be converted.
wanted to. This does a rough check to make sure that the value contains only
valid base64 characters before trying to convert it. TODO: there could be more
work done here to only add values that once decoded, end up as ascii
characters (since we're working in python), but that can be future work.
are backwards. TODO: should change the other as well
Tests pass locally (and in the second commit), and don't pass even when the interview steps are kept, meaning
that the fix does actually fix a DA related bug. The third commit (last one as of writing) fails because it includes extra steps not yet merged in ALAutomatedTestsingTests.