You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@metron.apache.org by GitBox <gi...@apache.org> on 2019/10/07 10:55:31 UTC

[GitHub] [metron] ruffle1986 opened a new pull request #1527: METRON-2276: [UI] Performance: Switching back from manual filtering fires a "full query"

ruffle1986 opened a new pull request #1527: METRON-2276: [UI] Performance: Switching back from manual filtering fires a "full query"
URL: https://github.com/apache/metron/pull/1527
 
 
   ## Contributor Comments
   
   Link to the original Jira: https://issues.apache.org/jira/browse/METRON-2276
   
   The problem
   =========
   
   Every time the user switches back from "manual query mode" (to the query builder), we clear the filter and remove everything from the search input field. Even when there were something typed in the query builder previously.
   
   The solution
   =========
   
   It would be great if we save the value of the search field no matter if it's the query builder of the manual query mode. 
   So instead of firing a search event every time the user switches back from manual mode, let's just simply not fire it so we can see the previous value for the query builder. And vica versa.
   
   Previously, when the ace editor fired a "textChanged" event, we immediately called the search method. From now on, there's a different callback on the textChanged event which puts the application state in stale mode. 
   This is how it's supposed to work currently. On every change, the app should be in stale mode (there's a yellow warning message in the middle of the screen). The only way, to see the result of a new query is pressing the magnifier button (search button) on the right. 
   
   Previously that wasn't true if the user removed a query item in the query builder by pressing the tiny x icon on the right side of the query item. With this patch, this issue is going to be fixed too which means every time you remove a query item, the app is going to be in stale mode instead of performing a search immediately. 
   
   Although it's not the scope of this PR, I've found an issue with the manual query mode. The app generates the "raw" representation of the query builder's result when the user switches to manual mode BUT only for the first time when there's no previous manual mode value:
   
   https://github.com/apache/metron/blob/master/metron-interface/metron-alerts/src/app/alerts/alerts-list/query-builder.ts#L177-L179
   
   So when the user decides to go back to the query builder, change it and then go back to manual mode, the raw value in the manual mode will be no longer the proper representation of the query builder's value.
   
   Testing
   =====
   
   - Go the alerts-list page on the Alerts UI.
   - Hit the search button in order to get some result. (At the beginning, there's no result for the current value. The user has to press the search button. It should be in stale mode and show the yellow warning message)
   - Build a nice query with the query builder (for example by clicking on cell values in the table)
   - Go to manual mode and than go back to the query builder.
   - You should be able the see the previous value of the query builder and since nothing has changed, the application shouldn't be in stale mode. 
   - If you change the query with the builder by adding of removing new query items, it's gonna turn into stale mode obviously without searching immediately.
   
   Although it solves the performance issue and prevents the users from dosing the servers, there's some limitations in the manual query mode. When you change something in the manual query mode, the application doesn't turn into stale mode however it should. And every time you change something in the manual mode and go back to the query builder, currently, the app is unable to convert the manual query to the value that is accepted by query builder which means we are unable to switch back and forth and always see the proper value in the search field in the appropriate format based on if it's the query builder or the manual mode.
   
   ## Pull Request Checklist
   
   Thank you for submitting a contribution to Apache Metron.  
   Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions.  
   Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) for complete smoke testing guides.  
   
   
   In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:
   
   ### For all changes:
   - [X] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
   - [X] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   - [X] Has your PR been rebased against the latest commit within the target branch (typically master)?
   
   
   ### For code changes:
   - [X] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
   - [X] Have you included steps or a guide to how the change may be verified and tested manually?
   - [X] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
     ```
     mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh 
     ```
   
   - [X] Have you written or updated unit tests and or integration tests to verify your changes?
   - [X] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [X] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
   
   ### For documentation related changes:
   - [X] Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via `site-book/target/site/index.html`:
   
     ```
     cd site-book
     mvn site
     ```
   
   - [X] Have you ensured that any documentation diagrams have been updated, along with their source files, using [draw.io](https://www.draw.io/)? See [Metron Development Guidelines](https://cwiki.apache.org/confluence/display/METRON/Development+Guidelines) for instructions.
   
   #### Note:
   Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
   It is also recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services