Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP] web searchpanel: records count on folder #44490

Open
wants to merge 2 commits into
base: master
from

Conversation

@laa-odoo
Copy link
Contributor

laa-odoo commented Feb 3, 2020

Display the records count as text-muted on the far right of 'folder' item of the searchpanel.

TASK-ID: 2166814

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo robodoo added the seen 🙂 label Feb 3, 2020
@laa-odoo laa-odoo force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch 2 times, most recently from 78c24ff to dfea220 Feb 3, 2020
@C3POdoo C3POdoo added the RD label Feb 3, 2020
@laa-odoo laa-odoo force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch 11 times, most recently from 4d2aead to fdda112 Feb 3, 2020
@robodoo robodoo added the CI 🤖 label Feb 4, 2020
@laa-odoo laa-odoo force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch from fdda112 to de89549 Feb 5, 2020
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Feb 5, 2020
addons/web/models/models.py Outdated Show resolved Hide resolved
@@ -372,7 +374,8 @@ var SearchPanel = Widget.extend({
}
},
/**
* Fetch values for each category. This is done only once, at startup.
* Fetch values for each category. This is done at startup, and at each reload
* (when searchPanel domain changes).

This comment has been minimized.

Copy link
@aab-odoo

aab-odoo Feb 25, 2020

Contributor

Explain that this is necessary for counters?

This comment has been minimized.

Copy link
@Polymorphe57
addons/web/static/tests/helpers/mock_server.js Outdated Show resolved Hide resolved
@@ -1354,73 +1355,6 @@ QUnit.module('Views', {
kanban.destroy();
});

QUnit.test('only reload filters when domains change', async function (assert) {

This comment has been minimized.

Copy link
@aab-odoo

aab-odoo Feb 25, 2020

Contributor

maybe keep this test with disable_counter = 1

This comment has been minimized.

Copy link
@aab-odoo

aab-odoo Feb 25, 2020

Contributor

also add:

  • a test with selection field and counters
  • a test with selection field without counters (check RPCs)

This comment has been minimized.

Copy link
@Polymorphe57
addons/web/models/models.py Outdated Show resolved Hide resolved
@@ -383,21 +386,21 @@ var SearchPanel = Widget.extend({
var category = self.categories[categoryId];
var field = self.fields[category.fieldName];
var categoriesProm;
if (field.type === 'selection') {

This comment has been minimized.

Copy link
@aab-odoo

aab-odoo Feb 25, 2020

Contributor

only call server if counters are requested

This comment has been minimized.

Copy link
@Polymorphe57
addons/web/static/tests/helpers/mock_server.js Outdated Show resolved Hide resolved
addons/web/static/src/js/views/search_panel.js Outdated Show resolved Hide resolved
addons/web/models/models.py Outdated Show resolved Hide resolved
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch from de89549 to 3c95c77 Mar 11, 2020
@robodoo robodoo removed the CI 🤖 label Mar 11, 2020
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch from 49ed92a to 4bb006c Mar 13, 2020
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 13, 2020
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch from ed720f9 to 6818eff Mar 16, 2020
<i t-if="value.childrenIds.length" t-attf-class="fa fa-fw o_toggle_fold {{value.folded ? 'fa-caret-left' : 'fa-caret-down'}}"/>
<span t-if="value.count > 0" class="pull-right text-muted mt-1 small"><t t-esc="value.count"/></span>
<span t-if="category.disableCounters" class="pull-right text-muted mt-1 small">?</span>
<i t-if="value.childrenIds.length" t-attf-class="fa fa-fw o_toggle_fold {{value.folded ? 'fa-caret-right' : 'fa-caret-down'}}"/>

This comment has been minimized.

Copy link
@Polymorphe57

Polymorphe57 Mar 16, 2020

Contributor

If the display_name is very long and the value.childrenIds.length > 0,
bad style (carret is below the description). I think it should still be on same line as description

This comment has been minimized.

Copy link
@Polymorphe57

Polymorphe57 Mar 16, 2020

Contributor

fixed via scss

addons/web/static/tests/helpers/mock_server.js Outdated Show resolved Hide resolved
addons/web/static/tests/helpers/mock_server.js Outdated Show resolved Hide resolved
@@ -1354,73 +1355,6 @@ QUnit.module('Views', {
kanban.destroy();
});

QUnit.test('only reload filters when domains change', async function (assert) {

This comment has been minimized.

Copy link
@Polymorphe57
addons/web/static/tests/helpers/mock_server.js Outdated Show resolved Hide resolved
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch 3 times, most recently from b039e38 to 918b81e Mar 16, 2020
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch 2 times, most recently from 73a5713 to 8cc8a8d Mar 24, 2020
@Polymorphe57 Polymorphe57 requested review from rco-odoo and kebeclibre Mar 24, 2020
@robodoo robodoo added the CI 🤖 label Mar 24, 2020
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch from 8cc8a8d to badb2cb Mar 24, 2020
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 24, 2020
@Polymorphe57

This comment has been minimized.

Copy link
Contributor

Polymorphe57 commented Mar 24, 2020

Problem: only the direct parent has count incremented not all ancestors --> to fix

@robodoo robodoo removed the CI 🤖 label Mar 25, 2020
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch from 62e8d5f to aa5f390 Mar 25, 2020
@@ -260,6 +261,10 @@ def group_id_name(value):

records = Comodel.with_context(hierarchical_naming=False).search_read([], field_names)

forest = defaultdict(lambda: {

This comment has been minimized.

Copy link
@kebeclibre

kebeclibre Mar 25, 2020

Contributor

lol, tree is clearer ? but congrats on the pun !
EDIT: sorry, misunderstood the code below. But if we need to read the code to understand a variable .........
EDIT2: then flat_tree ? or mapped_flat_tree ?

This comment has been minimized.

Copy link
@Polymorphe57

Polymorphe57 Mar 25, 2020

Contributor

flat_forest then (we have many trees, many roots)

This comment has been minimized.

Copy link
@Polymorphe57

Polymorphe57 Mar 25, 2020

Contributor

The term forest is common I think

This comment has been minimized.

Copy link
@Polymorphe57

Polymorphe57 Mar 25, 2020

Contributor

or acyclic directed graph maybe?

category_values.append(values)

root_ids = [rec[0] for rec in forest.items() if rec[1]['is_root']]
forest['adjoint_id']['children_ids'] = root_ids

This comment has been minimized.

Copy link
@kebeclibre

kebeclibre Mar 25, 2020

Contributor

adjoint ? isn't it the root of roots ?

This comment has been minimized.

Copy link
@Polymorphe57

Polymorphe57 Mar 25, 2020

Contributor

It is like a virtual node that transform the forest (many disjoint trees) into an actual tree, this in order to have a nice function compute_global_count

@robodoo robodoo added the CI 🤖 label Mar 25, 2020
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch from aa5f390 to 2f3c069 Mar 25, 2020
@robodoo robodoo removed the CI 🤖 label Mar 25, 2020
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch from 2f3c069 to bedceb0 Mar 25, 2020
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 25, 2020
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch from b5ffe6c to 88164eb Mar 25, 2020
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 25, 2020
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch from 10b840d to ec85003 Mar 26, 2020
In the search panel, the record counts are now also available for
the fields with select="one" (if not disabled explicitely).

TASK-ID: 2166814

Co-authored-by: Mathieu Duckerts-Antoine <dam@odoo.com>
Co-authored-by: Alexis Lacroix <laa@odoo.com>
@Polymorphe57 Polymorphe57 force-pushed the odoo-dev:master-searchpanel-records-count-folder-laa branch from ec85003 to c3854ba Mar 26, 2020
return value and value[0]

# get counters
counters = defaultdict(lambda: 0)

This comment has been minimized.

Copy link
@Polymorphe57

Polymorphe57 Mar 26, 2020

Contributor

maybe a default dict is not a good idea, we set useless keys sometimes --> use get?

@robodoo robodoo added the CI 🤖 label Mar 27, 2020
# determine result
result = lazymapping(lambda id: {
'id': id,
'count': 0,

This comment has been minimized.

Copy link
@Polymorphe57

Polymorphe57 Mar 27, 2020

Contributor

why not counters[id] and adapt if count below?

This comment has been minimized.

Copy link
@Polymorphe57

Polymorphe57 Mar 27, 2020

Contributor

'count': counters[id] I mean

This comment has been minimized.

Copy link
@Polymorphe57

Polymorphe57 Mar 27, 2020

Contributor

This would avoid loop line 303

record_id = record['id']
count = counters[record_id]
if count:
while record_id:

This comment has been minimized.

Copy link
@Polymorphe57

Polymorphe57 Mar 27, 2020

Contributor

it would become

record_id = values.get('parent_id')
while record_id:
    ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.