Enable special characters and encode search string#119
Enable special characters and encode search string#119alfarok merged 2 commits intoDynamoDS:masterfrom alfarok:master
Conversation
|
@alfarok have you run the tests and made sure all are passing? |
|
@mjkkirschner some tests were actually failing prior to this PR... so those same tests are still failing but no new failures. Either way, looking into what's going on in there now. |
| <div | ||
| className="LibraryItemText" | ||
| > | ||
| DynamoText |
There was a problem hiding this comment.
@mjkkirschner These dependencies are referenced in the RawTypeData.json file and will cause the comparison to fail if they are not included here. I am not 100% what changed recently that would make this a new requirement, @ramramps do you have any ideas? As far as I can tell these packages have been in the RawTypeData.json file for a while.
There was a problem hiding this comment.
I am not quite sure about the comparison tests.. last time, it failed on my system. I think I made it to work.
There was a problem hiding this comment.
Everything passes if I include Clockwork and DynamoText in the snap. I wonder if changes with any of the element types made this a requirement at some point. It's hard to tell exactly when these tests broke without pulling old code and running them until everything passes
There was a problem hiding this comment.
I think they first start failing at #113 where tests were added around Add-ons. I think the above is requried after adding this section
| "RefreshLibraryViewRequestName": "refreshLibraryView", | ||
| "SearchTextUpdatedEventName": "searchTextUpdated", | ||
| "SectionIconClickedEventName": "sectionIconClicked", | ||
| "FilterCategoryEventName": "filterCategoryChange", |
There was a problem hiding this comment.
is there an issue right now? this was added for instrumentation...
There was a problem hiding this comment.
No it should be present but the order in which the objects are listed appears to influence the test results.
3 participants
QNTM-4042
Supplemental Dynamo PR
This PR enables all characters and spaces in search and encodes the string passed from librarie.js to Dynamo where the string is decoded and run through search. Previously, spaces were replaced in searches because the results were not accurately returned due to the string formatting. In a later PR all characters that were not
a-z0-9were removed from the query string to prevent special characters from crashing librarie. This PR should evade those issues. This PR also fixes test failures that were introduced prior.Looking forward we may want to consider modifying the search timeout logic to improve response fluidity.
Reviewers
@mjkkirschner
FYI
@Racel @jnealb @smangarole