pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] analytic: distribution widget plans

Browse Source 
There are a few issues solved in this commit
1. Fetch plans is called too many times (Journal items List view). This is not necessary as plans are only required if editing
2. Plans are not fetched again if the product/account changes. This is because the record referenced by props and nextProps is the same, hence a change is not detected.
3. In list views - analytic account data is retrieved for each instance of the widget. To reduce the calls, a batched read is used instead

Improve test

closes odoo/odoo#101803

X-original-commit: 01d7579cfc8bf441f03d4467ef34a2d3bae015a4
Signed-off-by: William André (wan) <wan@odoo.com>
This commit is contained in:
Habib (ayh)
2022-10-01 13:01:52 +02:00
parent 9087b3c2d8
commit 346a75d5ef
3 changed files with 108 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/account/views/account_move_views.xml
+1 -1
View File
@@ -1059,7 +1059,7 @@
<field name="analytic_distribution" widget="analytic_distribution"
groups="analytic.group_analytic_accounting"
optional="show"
options="{'product_field': 'product_id', 'account_field': 'account_id'}"
options="{'account_field': 'account_id'}"
business_domain_compute="parent.move_type in ['out_invoice', 'out_refund', 'out_receipt'] and 'sale' or parent.move_type in ['in_invoice', 'in_refund', 'in_receipt'] and 'purchase' or 'general'"/>
<field name="date_maturity"
optional="hide"
addons/analytic/static/src/components/analytic_distribution/analytic_distribution.js
+37 -28
View File
@@ -6,6 +6,7 @@ import { evaluateExpr } from "@web/core/py_js/py";
import { getNextTabableElement, getPreviousTabableElement } from "@web/core/utils/ui";
import { usePosition } from "@web/core/position_hook";
import { getActiveHotkey } from "@web/core/hotkeys/hotkey_service";
import { shallowEqual } from "@web/core/utils/arrays";
import { AnalyticAutoComplete } from "../autocomplete/autocomplete";
import { standardFieldProps } from "@web/views/fields/standard_field_props";
@@ -59,11 +60,16 @@ export class AnalyticDistribution extends Component {
},
fieldString: this.env._t("Analytic Distribution Template"),
});
this.allPlans = [];
this.lastAccount = this.props.account_field ? this.props.record.data[this.props.account_field] : false;
this.lastProduct = this.props.product_field ? this.props.record.data[this.props.product_field] : false;
}
// Lifecycle
async willStart() {
await this.fetchAllPlans(this.props);
if (this.editingRecord) {
await this.fetchAllPlans(this.props);
}
await this.formatData(this.props);
}
@@ -72,36 +78,39 @@ export class AnalyticDistribution extends Component {
// and thus different applicabilities apply
// or a model applies that contains unavailable plans
// This should only execute when these fields have changed, therefore we use the `_field` props.
// (consider including the plans in the computed json, python side)
const valueChanged = JSON.stringify(this.props.value) !== JSON.stringify(nextProps.value);
if (this.applicabilityParamsChanged(nextProps) || valueChanged) {
await this.fetchAllPlans(nextProps);
const currentAccount = this.props.account_field ? this.props.record.data[this.props.account_field] : false;
const currentProduct = this.props.product_field ? this.props.record.data[this.props.product_field] : false;
const accountChanged = !shallowEqual(this.lastAccount, currentAccount);
const productChanged = !shallowEqual(this.lastProduct, currentProduct);
if (valueChanged || accountChanged || productChanged) {
if (!this.props.force_applicability) {
await this.fetchAllPlans(nextProps);
}
this.lastAccount = accountChanged && currentAccount || this.lastAccount;
this.lastProduct = productChanged && currentProduct || this.lastProduct;
await this.formatData(nextProps);
}
}
applicabilityParamsChanged(nextProps) {
if (this.props.force_applicability) {
return false;
}
if (this.props.account_field && this.props.record.data[this.props.account_field] !== nextProps.record.data[this.props.account_field] ||
this.props.product_field && this.props.record.data[this.props.product_field] !== nextProps.record.data[this.props.product_field]) {
return true;
}
return false;
patched() {
this.focusToSelector();
}
async formatData(nextProps) {
const data = nextProps.value;
const analytic_account_ids = Object.keys(data);
const analytic_account_ids = Object.keys(data).map((id) => parseInt(id));
const records = analytic_account_ids.length ? await this.fetchAnalyticAccounts([["id", "in", analytic_account_ids]]) : [];
if (records.length < data.length) {
console.log('removing tags... value should be updated');
}
let res = Object.assign({}, ...this.allPlans.map((plan) => ({[plan.id]: {...plan, distribution: []}})));
let widgetData = Object.assign({}, ...this.allPlans.map((plan) => ({[plan.id]: {...plan, distribution: []}})));
records.map((record) => {
res[record.root_plan_id[0]].distribution.push({
if (!widgetData[record.root_plan_id[0]]) {
// plans might not have been retrieved
widgetData[record.root_plan_id[0]] = { distribution: [] }
}
widgetData[record.root_plan_id[0]].distribution.push({
analytic_account_id: record.id,
percentage: data[record.id],
id: this.nextId++,
@@ -111,11 +120,7 @@ export class AnalyticDistribution extends Component {
});
});
this.state.list = res;
}
patched() {
this.focusToSelector();
this.state.list = widgetData;
}
// ORM
@@ -158,6 +163,10 @@ export class AnalyticDistribution extends Component {
if (limit) {
args['limit'] = limit;
}
if (domain.length === 1 && domain[0][0] === "id") {
//batch these orm calls
return await this.props.record.model.orm.read("account.analytic.account", domain[0][2], args.fields, {});
}
return await this.orm.call("account.analytic.account", "search_read", [], args);
}
@@ -402,17 +411,13 @@ export class AnalyticDistribution extends Component {
validate() {
for (const group_id in this.list) {
if (this.groupStatus(group_id) === 'red') {
this.invalidate();
this.props.record.setInvalidField(this.props.name);
return false;
}
}
return true;
}
invalidate() {
this.props.record.setInvalidField(this.props.name);
}
async save() {
const currentDistribution = this.listForJson;
const dataToSave = currentDistribution;
@@ -441,7 +446,11 @@ export class AnalyticDistribution extends Component {
this.state.showDropdown = false;
}
openAnalyticEditor() {
async openAnalyticEditor() {
if (!this.allPlans.length) {
await this.fetchAllPlans(this.props);
await this.formatData(this.props);
}
this.autoFill();
const incompletePlan = this.firstIncompletePlanId;
this.setFocusSelector(incompletePlan ? `#plan_${incompletePlan} .incomplete`: ".analytic_json_popup");
addons/analytic/static/tests/analytic_distribution_tests.js
+70 -49
View File
@@ -2,7 +2,10 @@
import {
click,
editInput,
getFixture,
nextTick,
triggerHotkey,
} from "@web/../tests/helpers/utils";
import { makeView, setupViewRegistries } from "@web/../tests/views/helpers";
@@ -14,18 +17,18 @@ QUnit.module("Analytic", (hooks) => {
target = getFixture();
serverData = {
models: {
analytic_account: {
"account.analytic.account": {
fields: {
plan_id: { string: "Plan", type: "many2one", relation: "plan" },
root_plan_id: { string: "Root Plan", type: "many2one", relation: "plan" },
color: { string: "Color", type: "integer" },
},
records: [
{id: 1, color: 1, root_plan_id: 1, plan_id: 1, name: "RD" },
{id: 2, color: 1, root_plan_id: 1, plan_id: 1, name: "HR" },
{id: 3, color: 1, root_plan_id: 1, plan_id: 1, name: "FI" },
{id: 4, color: 2, root_plan_id: 2, plan_id: 2, name: "Time Off" },
{id: 5, color: 2, root_plan_id: 2, plan_id: 2, name: "Operating Costs" },
{id: 1, color: 1, root_plan_id: 2, plan_id: 2, name: "RD" },
{id: 2, color: 1, root_plan_id: 2, plan_id: 2, name: "HR" },
{id: 3, color: 1, root_plan_id: 2, plan_id: 2, name: "FI" },
{id: 4, color: 2, root_plan_id: 1, plan_id: 1, name: "Time Off" },
{id: 5, color: 2, root_plan_id: 1, plan_id: 1, name: "Operating Costs" },
{id: 6, color: 6, root_plan_id: 4, plan_id: 4, name: "Incognito" },
{id: 7, color: 5, root_plan_id: 5, plan_id: 5, name: "Belgium" },
{id: 8, color: 6, root_plan_id: 5, plan_id: 6, name: "Brussels" },
@@ -45,14 +48,15 @@ QUnit.module("Analytic", (hooks) => {
},
color: { string: "Color", type: "integer" },
all_account_count: { type: "integer" },
parent_id: { type: "many2one", relation: "plan" },
},
records: [
{ id: 1, name: 'Departments', applicability: "mandatory" },
{ id: 2, name: 'Internal', applicability: "optional", all_account_count: 2 },
{ id: 1, name: 'Internal', applicability: "optional", all_account_count: 2 },
{ id: 2, name: 'Departments', applicability: "mandatory", all_account_count: 3 },
{ id: 3, name: 'Projects', applicability: "optional" },
{ id: 4, name: 'Hidden', applicability: "unavailable" },
{ id: 5, name: "Country", applicability: "optional"},
{ id: 6, name: "City", applicability: "optional"},
{ id: 4, name: 'Hidden', applicability: "unavailable", all_account_count: 1 },
{ id: 5, name: "Country", applicability: "optional", all_account_count: 3 },
{ id: 6, name: "City", applicability: "optional", all_account_count: 2, parent_id: 5 },
],
},
aml: {
@@ -86,7 +90,7 @@ QUnit.module("Analytic", (hooks) => {
QUnit.module("AnalyticDistribution");
QUnit.test("field in form view should open and close", async function (assert) {
QUnit.test("field in form view basic features", async function (assert) {
await makeView({
type: "form",
resModel: "aml",
@@ -103,59 +107,76 @@ QUnit.module("Analytic", (hooks) => {
</sheet>
</form>`,
mockRPC(route, { kwargs, method, model }) {
if (method === "search_read" && model === "account.analytic.plan") {
return Promise.resolve(serverData.models['plan'].records);
} else if (method === "search_read" && model === "account.analytic.account") {
if (kwargs.domain[0][0] == "id" && kwargs.domain[0][1] == "in") {
const required_ids = kwargs.domain[0][2].map((id) => parseInt(id));
const analytic_accs = serverData.models['analytic_account'].records.filter((r) => required_ids.includes(r.id));
const accs_with_plan = analytic_accs.map(
(r) => (
{
...r,
root_plan_id: Object.values(serverData.models['plan'].records.find((p) => p.id === r.root_plan_id))
}
));
return Promise.resolve(accs_with_plan);
}
return Promise.resolve(serverData.models['analytic_account'].records);
} else if (method === "get_relevant_plans" && model === "account.analytic.plan") {
return Promise.resolve(serverData.models['plan'].records);
if (method === "get_relevant_plans" && model === "account.analytic.plan") {
return Promise.resolve(
serverData.models['plan'].records.filter((r) => !r.parent_id && r.applicability !== "unavailable")
);
}
},
});
assert.containsOnce(target, ".analytic_distribution", "widget should be visible");
assert.containsN(target, ".badge", 2, "should contain 2 tags");
assert.strictEqual(
target.querySelector(".badge .o_tag_badge_text").textContent,
"RD 30.3%",
assert.strictEqual(target.querySelector(".badge .o_tag_badge_text").textContent, "RD 30.3%",
"should have rendered 'RD 30.3%'"
);
assert.strictEqual(
target.querySelectorAll(".badge .o_tag_badge_text")[1].textContent,
"FI 69.7%",
assert.strictEqual(target.querySelectorAll(".badge .o_tag_badge_text")[1].textContent, "FI 69.7%",
"should have rendered 'FI 69.7%'"
);
assert.containsN(
target,
".o_delete",
2,
"tags should contain a delete button"
assert.containsN(target, ".o_delete", 2, "tags should contain a delete button");
let badge1 = target.querySelector('.badge');
await click(badge1, ".o_tag_badge_text");
assert.containsN(target, ".analytic_distribution_popup", 1, "popup should be visible");
let popup = target.querySelector('.analytic_distribution_popup');
let planTable = popup.querySelectorAll('table')[0];
assert.strictEqual(planTable.id, "plan_2", "mandatory plan appears first");
assert.containsN(planTable, 'tr', 4,
"first plan contains 4 rows including: title, 2 tags, add a line"
);
assert.strictEqual(document.activeElement, planTable.querySelector('input'),
"focus is on the first analytic account"
);
const badge1 = target.querySelector('.badge');
await click(badge1, ".o_tag_badge_text");
assert.containsN(
target,
".analytic_distribution_popup",
1,
"popup should be visible"
triggerHotkey("Tab");
const input = document.activeElement;
await editInput(input, null, "19");
assert.containsOnce(planTable, '.o_analytic_status_orange', "Mandatory plan has incomplete status");
let incompleteInputName = planTable.querySelector('tr.incomplete .o_analytic_account_name input');
assert.strictEqual(document.activeElement, incompleteInputName,
"focus is on the first incomplete tag"
);
triggerHotkey("Escape");
await nextTick();
assert.containsNone(target, '.analytic_distribution_popup', "The popup should be closed and invalid");
assert.containsOnce(target, "div .o_field_analytic_distribution.o_field_invalid", "Field should be invalid");
triggerHotkey("arrowdown"); //opens the popup again
await nextTick();
popup = target.querySelector('.analytic_distribution_popup');
planTable = popup.querySelectorAll('table')[0];
incompleteInputName = planTable.querySelector('tr.incomplete .o_analytic_account_name input');
assert.strictEqual(document.activeElement, incompleteInputName,
"focus returns to the first incomplete tag"
);
triggerHotkey("arrowdown");
await nextTick();
triggerHotkey("Tab");
await nextTick();
assert.strictEqual(document.activeElement.value, "11.3%", "remainder percentage is prepopulated");
await click(target, '.fa-close');
assert.containsNone(target, '.analytic_distribution_popup', "The popup should be closed");
assert.containsNone(target, '.o_field_invalid', "Distribution is valid");
assert.containsN(target, ".badge", 3, "should contain 3 tags");
});
});