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_timeline - support multiple group-by levels and allow to group by m2m field #3006

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

cuongnmtm
Copy link
Contributor

@cuongnmtm cuongnmtm commented Dec 2, 2024

Allowing to add multiple group-by levels

image

Test cases:

  • group by one field (group by m2o, selection, integer, m2m, date, ...)
  • group by multiple fields
  • group by 0 field: back to default group by
  • create a new record
  • drag and drop record to update
  • click on the group by to view the group by record detail
  • when group by m2m field, record is displayed multiple times if linked to multiple m2m records
  • when drag and drop, the value of the m2m field should not be updated as there is no best way to handle this
  • when creating a new record, set the default value for all grouped fields
  • handle group by non-relation fields: click on the group of selection or char field do not popup record form view to avoid error.
  • dependency_arrow

@OCA-git-bot
Copy link
Contributor

Hi @tarteo,
some modules you are maintaining are being modified, check this out!

@cuongnmtm cuongnmtm force-pushed the web_timeline-multi-group-by branch 2 times, most recently from 5a5f65b to 8cf395a Compare December 2, 2024 12:45
@pedrobaeza
Copy link
Member

Can you please put an screenshot of the feature?

@cuongnmtm
Copy link
Contributor Author

@pedrobaeza I have included a screenshot of the feature in the PR description.

@pedrobaeza pedrobaeza added this to the 16.0 milestone Dec 2, 2024
@pedrobaeza
Copy link
Member

Great improvement!

@cuongnmtm cuongnmtm changed the title [IMP] web_timeline - support multi group by level [IMP] web_timeline - support multiple group-by levels and allow to group by m2m field Dec 2, 2024
@cuongnmtm cuongnmtm force-pushed the web_timeline-multi-group-by branch 2 times, most recently from 7e7bc6e to e34aeec Compare December 2, 2024 20:40
Copy link
Member

@tarteo tarteo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!
I get an error when I quickly add and remove a group (by double clicking). However if I click normally I don't get an error. There's probably a race condition somewhere

Error (I have assets debugging enabled):
UncaughtPromiseError > TypeError Uncaught Promise > Cannot read properties of undefined (reading 'relation') TypeError: Cannot read properties of undefined (reading 'relation') at Class.split_groups (http://localhost:8069/web/assets/debug/web.assets_backend.js:190546:71) (/web_timeline/static/src/js/timeline_renderer.js:509)

@cuongnmtm cuongnmtm force-pushed the web_timeline-multi-group-by branch 3 times, most recently from 8279013 to 0a27fb8 Compare December 6, 2024 12:40
@cuongnmtm
Copy link
Contributor Author

@tarteo I cannot reproduce your issue, but I found another related defect and fixed it (error when clicking on the group when group by non-relation field). Could you please help to check again? Thanks.

And other things had been done (inthe description).

@cuongnmtm
Copy link
Contributor Author

For non-technical who wish to test this feature, you can use the XML view arch below to create a timeline view for the project model and modify the project window action to include the timeline view mode for testing.

<timeline date_start="date_start" date_stop="date" string="Projects" default_group_by="user_id,partner_id,privacy_visibility" mode="week" event_open_popup="true" stack="true">
    <field name="user_id"/>
    <field name="privacy_visibility"/>
    <field name="tag_ids"/>
    <field name="name"/>
    <templates>
        <t t-name="timeline-item">
            <div t-att-title="record.name">
                <small>
                    <div>
                      <span>
                          <t t-esc="record.name"/>
                      </span>
                    </div>
                    <div>
                      PM:
                      <span>
                          <t t-esc="record.user_id[1]"/>
                      </span>
                    </div>
                  
                </small>
            </div>
        </t>
    </templates>
</timeline>

@pedrobaeza
Copy link
Member

We added on the scheduled actions the timeline view as demo data for demonstration purposes without requiring extra modules. Maybe you can modify that demo view to showcase this feature.

@cuongnmtm cuongnmtm force-pushed the web_timeline-multi-group-by branch from 0a27fb8 to f7b6591 Compare December 6, 2024 14:19
@cuongnmtm
Copy link
Contributor Author

We added on the scheduled actions the timeline view as demo data for demonstration purposes without requiring extra modules. Maybe you can modify that demo view to showcase this feature.

I missed that. Thanks.

@antoniodavid
Copy link

Awesome @cuongnmtm. It would be great to update the version of the addon.

@pedrobaeza
Copy link
Member

Awesome @cuongnmtm. It would be great to update the version of the addon.

No, that's done on merge by the bot, when we issue the command.

@cuongnmtm cuongnmtm force-pushed the web_timeline-multi-group-by branch from f7b6591 to 0bb5942 Compare December 13, 2024 08:53
Copy link

@minhthie minhthie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional test: LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants