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

[16.0][FIX]mail_activity_done: Fix counters and activity_state search #1507

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

Conversation

jesusVMayor
Copy link
Member

@jesusVMayor jesusVMayor commented Nov 21, 2024

This PR includes 2 changes:

  • Remove systray_get_activities overrride, its not necessary since odoo/odoo@bd4e8aa
  • Override _search_activity_state adding and mail_activity.done=False to WHERE clause in the query to allow properly search by this field.

Fixes: #1327 CC: @gurneyalex @navarrorico

@jesusVMayor jesusVMayor force-pushed the 16.0_fix_activity_done branch 2 times, most recently from ea4838f to df64eed Compare November 21, 2024 13:51
@jesusVMayor
Copy link
Member Author

Fixed tests.

@jesusVMayor jesusVMayor force-pushed the 16.0_fix_activity_done branch from df64eed to 9ab7f24 Compare November 22, 2024 10:44
@jesusVMayor
Copy link
Member Author

I have detected a new bug, the _read_progress_bar inherit was not properly working and the count of activities in the kanban view was wrong. We should modify the subquery where clause

Copy link
Member

@hbrunn hbrunn left a comment

Choose a reason for hiding this comment

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

looks like you'll have to update the test too - I think it's correct to expect a 0 here

@jesusVMayor jesusVMayor force-pushed the 16.0_fix_activity_done branch from 1f68c55 to 1f6bc04 Compare December 30, 2024 15:50
@jesusVMayor
Copy link
Member Author

Seems a problem with the tests because the function executes the query directly. Testing in shell, if i make the commit after the _action_done, the result is 0. It also happens in the test_activity_state_search.

I changed the tests to move the second activity to demo file, assigned to another partner, and created already done.

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Thanks! Sounds like your test issues could perhaps have been solved with a flush_recordset on the activity.
Do you want to squash your commits in one or maybe two commits? I already picked what remains valid in 18.0 in OCA/mail#31

@jesusVMayor jesusVMayor force-pushed the 16.0_fix_activity_done branch from 1f6bc04 to 4da6a2e Compare February 13, 2025 13:30
@jesusVMayor
Copy link
Member Author

Done

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

If I understand correctly, the tests now depend on the demo data. This is against the current grain of making tests less dependent on demo data. The tests are also more reliable if they are for a newly created user. Otherwise, the tests could fail because of demo activities from unrelated modules, or previous activity on a reused database. Can you bring back the test user?

In odoo 16 the systray_get_activities function uses the ORM, there is no need to override it.

Also fixes the activity count in kanban views.
@jesusVMayor jesusVMayor force-pushed the 16.0_fix_activity_done branch from 4da6a2e to f1fffe2 Compare February 14, 2025 15:38
@jesusVMayor
Copy link
Member Author

I made some changes in tests:

  • Removed demo data in tests, and use flush_recordset to avoid problems.
  • Added tz to new user, odoo base _search_activity_state fails if the user has no tz.
  • Create test partner with the test user, it causes some security rules problems if is created without it.

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.

mail_activity_done: wrong counters
3 participants