This PR fixes 2 issues that were picked up by users for the Scheduled Data Explorer Report automation script and a couple of small improvements to better match the format of manual data explorer query results.
The first issue is that within result_to_markdown the colrender contains null values, and we are currently checking length (previously treated as a packed array but it is actually a sparse array). Therefore we can check if the current index of the array is null rather than checking the size of the array.
The second issue addresses the blank query_params field. When the data explorer script does not require any params to be passed in via the automation script then it will have a nil value, however it should be defaulted to {} within the plugin.
To improve formatting the markdown table for PMs is now aligned to left and where values are substituted (for example user_id becomes username) we then include the id within brackets, for example:
user_id becomes username (user_id)
Data Explorer can run arbitrary SQL queries which can be costly for us if over-used. Because of that we want to add the ability to rate limit the query run endpoint, in particular when requested programmatically using API.
This commit introduces a rate limit to the `QueryController#run` endpoint. It heavily leans on the existing `RateLimiter` implementation, and the ability of `ApplicationController` to turn rate limit exceptions into nicely formatted JSON responses.
The rate limit (per 10 seconds) can be configured through the global setting `max_data_explorer_api_reqs_per_10_seconds`, and defaults to 2.
Handling can be configured through `max_data_explorer_api_req_mode`, and can be set to warn, block, or both warn and block. We will default to warn for now and monitor the logs for a while.
This feature enables admins to create reports automatically based on a recurring schedule.
It introduces a new automation script that includes the new email_group_user field added to discourse-automation, along with a query_id and query_params to pass in parameters to the existing data explorer query.
The output of the report will be sent via pm (as a markdown table) to the recipients entered within the automation script.
The automation (supports individual users, email addresses and groups).
This commit updates the plugin to the latest guidelines, as shown in
discourse-plugin-skeleton, which involves moving a lot of the code to
dedicated files, use proper namespaces, use the autoloader as much as
possible, etc.
Followup to 360d0dde650704a0f01fd6d8b525e933b1d7fcf2,
this causes other plugin tests to fail because
`DiscoursePluginRegistry.reset!` is
a shotgun. We can use the more surgical version
`DiscoursePluginRegistry.reset_register!(:bookmarkables)`
instead.
- Require query name is present
- Ensure all routes are treated by default as .json, so errors flow correctly
- Remove superflous save/cancel controls from group settings
- Remove group control when item is destroyed
- Disable editing of query when it is deleted
Co-authored-by: Osama Sayegh <[email protected]>
Before this fix, the use of PG template patterns containing ":" or the
use of "?" in comments in the SQL will result in an error being raised
because `DB.param_encoder.encode` calls ActiveRecord's `sanitize_sql_array` which is
meant for SQL fragments and not an entire SQL string.
Instead we change data-explorer to use `MiniSql::InlineParamEncoder`
instead which takes into account of template patterns and does not trip
on `?` which is a special param encoding character used by ActiveRecord.
* FIX: allow groups to access system queries (without having to run the query once first)
Bug is: Trying to allow a group to access a system query results in a Discourse::NotFound unless the query is run first.
Cause:
- System queries don't exist in the database by default
- update calls set_query before action
- set_query searches the database for the system query with Query.find_by(:id), which will not exist by default.
- running system queries first fixes this because Query.find is overridden to include system queries (Queries.default) in its results, avoiding the Discourse::NotFound.
Solution: use the overridden Query.find in set_query to include system queries in the search, instead of Query.find_by(:id)
* Added test for fixing allowing groups to access system query.
* Fixed test formatting.
Previously the `QUERY_RESULT_DEFAULT_LIMIT` const was used
to limit the number of query results. This commit adds the
`data_explorer_query_result_limit` site setting which defaults
to 1000 and has a max of 10000 which matches the const
`QUERY_RESULT_MAX_LIMIT`.
Previously this was adding an extra AJAX request to check if the group had any queries available. Now a boolean is included in the group serializer, so there is no need for the extra request.
Removing this ajax request will also stop other plugin JS integration tests from failing when the data-explorer plugin is installed.
This commit also fixes the HTML markup of the tab, so that it doesn't have a <ul> nested inside the existing <ul>. Also adds an icon for good measure.
This rake task is responsible for fixing broken ids - move them back to the original values - ones before migration to a separate table.
Rake task can only fix queries with unique names.
In the test, we are creating duplicated queries and using `original_id` 4. Spec is ensuring that after rake tasks valuation, queries id is different than `4`
However, spec run in random order may create a query with id 4 and in that case, the spec would fail.
Therefore, when we create queries for test purpose, we should control what ids they are given to avoid conflict.
Reintroduces the migration removed in `cdfc5d4`, and fixes it to work with `query_groups`. Since the code is fairly complex, I moved into a rake task so it can be tested and make sure it works.
David Battersby
Martin Brennan
Joffrey JAFFEUX
Ted Johansson
Alan Guo Xiang Tan
Bianca Nenciu
Krzysztof Kotlarek
David Taylor
SamandOsama Sayegh
Frank
Jarek Radosz
Martin Brennan
Arpit Jalan
Roman Rizzi