-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[MERGE] project: improve burndown chart performance
Purpose of this PR =============== The Burndown Chart report was very slow on big databases as it was not possible for Postgresql to optimize the query as it was based on a view that was using several generate series. This commit aims to improve the performance by injecting the constraints at a lower level than it was in the past, lowering the amount of data processed in the higher level of the query. /!\ Important note ------------------ Overwriting the `read_group_raw` is really not a good practice and should be avoided in most case. If you fall on this implementation by grepping the source code, please be advised that this is not the right way of doing things. Implementation details ---------------------- - The report is now run by generating the `SQL` that is executed by the `read_group_raw`. This allows inserting `SQL` constraints at a lower level and simnifically improves performance. As there is no other way to do it, the code is unfortunately a modified copy of the actual `read_group_raw`. - The pivot view has been removed as it had no meaning and was creating confusing data. - The `Group By` menu has been limited to `stage_id` and `date` as bringing more data trough the different `GROUP BY` statements up to the higher level is costly. Further more, additional `Group By` did not bring added value as the Chart was less readable. - The JS code has been adapted in order to force a group by both `stage_id` and `date` so that the date displayed is always making sense. - The sort ascending and descending options have been removed as creating confusing data. - The compare with previous period has also been removed as the chart only really make sense when seen chronologically. - A lot of tests have been added in order to ensure that changes that would be harmful for the report will trigger test fails. task-2845729 -- I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr closes odoo#93225 Signed-off-by: Yannick Tivisse (yti) <[email protected]>
- Loading branch information
Showing
19 changed files
with
1,192 additions
and
372 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
606 changes: 420 additions & 186 deletions
606
addons/project/report/project_task_burndown_chart_report.py
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14 changes: 0 additions & 14 deletions
14
addons/project/static/src/burndown_chart/burndown_chart_pivot_model.js
This file was deleted.
Oops, something went wrong.
10 changes: 0 additions & 10 deletions
10
addons/project/static/src/burndown_chart/burndown_chart_pivot_view.js
This file was deleted.
Oops, something went wrong.
135 changes: 135 additions & 0 deletions
135
addons/project/static/src/burndown_chart/burndown_chart_search_model.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| /** @odoo-module */ | ||
|
|
||
| import { useService } from "@web/core/utils/hooks"; | ||
| import { SearchModel } from "@web/search/search_model"; | ||
|
|
||
|
|
||
| export class BurndownChartSearchModel extends SearchModel { | ||
|
|
||
| /** | ||
| * @override | ||
| */ | ||
| setup(services) { | ||
| this.notificationService = useService("notification"); | ||
| super.setup(...arguments); | ||
| } | ||
|
|
||
| /** | ||
| * @override | ||
| */ | ||
| async load(config) { | ||
| await super.load(...arguments); | ||
| // Store date and stage_id searchItemId in the SearchModel for reuse in other functions. | ||
| for (const searchItem of Object.values(this.searchItems)) { | ||
| if (['dateGroupBy', 'groupBy'].includes(searchItem.type)) { | ||
| if (this.stageIdSearchItemId && this.dateSearchItemId) { | ||
| return; | ||
| } | ||
| switch (searchItem.fieldName) { | ||
| case 'date': | ||
| this.dateSearchItemId = searchItem.id; | ||
| break; | ||
| case 'stage_id': | ||
| this.stageIdSearchItemId = searchItem.id; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @override | ||
| */ | ||
| deactivateGroup(groupId) { | ||
| // Prevent removing Date & Stage group by from the search | ||
| if (this.searchItems[this.stageIdSearchItemId].groupId == groupId && this.searchItems[this.dateSearchItemId].groupId) { | ||
| this._addGroupByNotification(this.env._t("Date and Stage")); | ||
| return; | ||
| } | ||
| super.deactivateGroup(groupId); | ||
| } | ||
|
|
||
| /** | ||
| * @override | ||
| */ | ||
| toggleDateGroupBy(searchItemId, intervalId) { | ||
| // Ensure that there is always one and only one date group by selected. | ||
| if (searchItemId === this.dateSearchItemId) { | ||
| let filtered_query = []; | ||
| let triggerNotification = false; | ||
| for (const queryElem of this.query) { | ||
| if (queryElem.searchItemId !== searchItemId) { | ||
| filtered_query.push(queryElem); | ||
| } else if (queryElem.intervalId === intervalId) { | ||
| triggerNotification = true; | ||
| } | ||
| } | ||
| if (filtered_query.length !== this.query.length) { | ||
| this.query = filtered_query; | ||
| if (triggerNotification) { | ||
| this._addGroupByNotification(this.env._t("Date")); | ||
| } | ||
| } | ||
| } | ||
| super.toggleDateGroupBy(...arguments); | ||
| } | ||
|
|
||
| /** | ||
| * @override | ||
| */ | ||
| toggleSearchItem(searchItemId) { | ||
| // Ensure that stage_id is always selected. | ||
| if (searchItemId === this.stageIdSearchItemId | ||
| && this.query.some(queryElem => queryElem.searchItemId === searchItemId)) { | ||
| this._addGroupByNotification(this.env._t("Stage")); | ||
| return; | ||
| } | ||
| super.toggleSearchItem(...arguments); | ||
| } | ||
|
|
||
| /** | ||
| * Adds a notification relative to the group by constraint of the Burndown Chart. | ||
| * @param fieldName The field name(s) the notification has to be related to. | ||
| * @private | ||
| */ | ||
| _addGroupByNotification(fieldName) { | ||
| const notif = this.env._t("The Burndown Chart must be grouped by"); | ||
| this.notificationService.add( | ||
| `${notif} ${fieldName}`, | ||
| { type: "danger" } | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * @override | ||
| */ | ||
| async _notify() { | ||
| // Ensure that we always group by date firstly and by stage_id secondly | ||
| let stageIdIndex = -1; | ||
| let dateIndex = -1; | ||
| for (const [index, queryElem] of this.query.entries()) { | ||
| if (stageIdIndex !== -1 && dateIndex !== -1) { | ||
| break; | ||
| } | ||
| switch (queryElem.searchItemId) { | ||
| case this.dateSearchItemId: | ||
| dateIndex = index; | ||
| break; | ||
| case this.stageIdSearchItemId: | ||
| stageIdIndex = index; | ||
| break; | ||
| } | ||
| } | ||
| if (stageIdIndex > 0) { | ||
| if (stageIdIndex > dateIndex) { | ||
| dateIndex += 1; | ||
| } | ||
| this.query.splice(0, 0, this.query.splice(stageIdIndex, 1)[0]); | ||
| } | ||
| if (dateIndex > 0) { | ||
| this.query.splice(0, 0, this.query.splice(dateIndex, 1)[0]); | ||
| } | ||
| await super._notify(...arguments); | ||
| } | ||
|
|
||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.