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] [REF] [MIG] coopiteasy_custom #106

Open
wants to merge 43 commits into
base: 16.0
Choose a base branch
from

Conversation

victor-champonnois
Copy link
Member

@victor-champonnois victor-champonnois commented Jul 26, 2023

  1. Refactoring coopiteasy_custom :
    • put two functionalities in their own modules : task_author_id and sort_project_by_timesheet_line_count
    • linked_tasks functionality is now standard as "dependent tasks". Need to write a script to migrate linked_tasks to dependent tasks.
      • Note : I could also have used subtasks but the functional intention is different, subtasks tends to be in the same project by default (unless the subtask's project is already set). Subtasks are child tasks, while dependent tasks are tasks that must be done before the other task. Since we used linked_tasks mainly for mise en prod tasks, dependent tasks make more sense.
    • other custom fields (reviewer,tester,priority) are abandoned because they are not used
    • PR field is now in a OCA module
    • not searching in closed project can be replaced by archiving project

cf internal discussion : https://docs.google.com/spreadsheets/d/1Bcyhw0BJRztjKtIGJmnnpBXLGv5Ctb7l378MqoyjgFQ/edit#gid=2001558567

  1. And porting to 16.0

Note : I didn't make a PR to 12 because it is useless. This refactoring is only due to the migration because we want to reduce the codebase.

@carmenbianca I am not sure if I should write a script to rename the cron's XML IDs. Do you have an opinion ? (not nessary anymore)

@robinkeunen I am really wondering whether it is worth it to continue using "sort_project_by_timesheet_line_count". Without this module, with Odoo standard, we can order the projects via total_timesheet_time or by task_count (just by configuring the DB via the interface). This module seems a bit complex to me compared to the functionality it provides.
If we don't want to use 'in interface' configuration because it could get lost in future migration, we could just set the model's order to total_timesheet_time or task_count in a module, without the need to compute "recent_timesheet_line_count".
What do you think ?

task : https://gestion.coopiteasy.be/web#id=10959&action=475&active_id=492&model=project.task&view_type=form&menu_id=

  • ne pas merger tant que les scripts linked_tasks et PR ne sont pas validés

@victor-champonnois victor-champonnois force-pushed the 16.0-mig-coopiteasy_custom branch from b3f2246 to 835732a Compare July 26, 2023 13:41
@victor-champonnois victor-champonnois changed the title 16.0 [MIG] coopiteasy_custom [16.0] [REF] [MIG] coopiteasy_custom Jul 26, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8a5913d) 87.80% compared to head (63cf94c) 88.37%.
Report is 1 commits behind head on 16.0.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             16.0     #106      +/-   ##
==========================================
+ Coverage   87.80%   88.37%   +0.56%     
==========================================
  Files           6        9       +3     
  Lines         123      129       +6     
  Branches       15       15              
==========================================
+ Hits          108      114       +6     
  Misses          9        9              
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@victor-champonnois victor-champonnois force-pushed the 16.0-mig-coopiteasy_custom branch 3 times, most recently from 1e9a96e to 75d3f12 Compare July 27, 2023 09:59
@robinkeunen
Copy link
Member

@victor-champonnois Ok to not porting sort_project_by_timesheet_line_count and go by your suggestion. If that's not enough, I'll complain and code it on the 3rd of septembre 2025

@victor-champonnois victor-champonnois force-pushed the 16.0-mig-coopiteasy_custom branch from adaeab3 to 60d654e Compare August 1, 2023 14:55
@victor-champonnois
Copy link
Member Author

@robinkeunen OK, I remove it then, thanks

@victor-champonnois
Copy link
Member Author

TODO : test the script to migrate linked_tasks to dependent tasks

@victor-champonnois
Copy link
Member Author

migration script needed for PR field

@carmenbianca
Copy link
Member

Maybe a helpful comment, but I wonder how migration scripts will work in this scenario. I envision the following execution flow:

  1. All installed modules are upgraded and run their migration scripts during the Odoo version migration.
  2. coopiteasy_custom is an installed module, so it is upgraded.
  3. As part of the upgrade, task_author_id is freshly installed.
  4. coopiteasy_custom migration scripts are run.
  5. task_author_id was freshly installed, so no migration scripts are run.

for task in env["project.task"].search([]):
if task.link_task_ids:
task.project_id.allow_task_dependencies = True
task.write({"depend_on_ids": task.link_task_ids})
Copy link
Member

Choose a reason for hiding this comment

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

@remytms will this work ? The field is not defined anymore un the module at execution time.

@robinkeunen robinkeunen force-pushed the 16.0-mig-coopiteasy_custom branch from 60d654e to f6207d3 Compare January 24, 2024 13:33
@robinkeunen robinkeunen self-requested a review January 24, 2024 13:34
@robinkeunen
Copy link
Member

@huguesdk @remytms @victor-champonnois Can I get some review please 🙏

@robinkeunen robinkeunen force-pushed the 16.0-mig-coopiteasy_custom branch from f6207d3 to 35fee29 Compare January 24, 2024 13:35
[ADD] cie_custom: uri and priority fields

Signed-off-by: Carmen Bianca Bakker <[email protected]>
blacken coopiteasy custom

wip

Signed-off-by: Carmen Bianca Bakker <[email protected]>
Signed-off-by: Carmen Bianca Bakker <[email protected]>
carmenbianca and others added 24 commits February 1, 2024 17:24
Signed-off-by: Carmen Bianca Bakker <[email protected]>
Signed-off-by: Carmen Bianca Bakker <[email protected]>
limit timesheet activity projects selection to open projects.
move analytic account custom order logic to project.

previously (in odoo 9), timesheet lines where directly linked to
analytic accounts. now (in odoo 12), they are linked to projects.
limit timesheet activity projects selection to open projects in task
timesheets and timesheet sheet details.
Signed-off-by: Carmen Bianca Bakker <[email protected]>
allow to directly select a task (from open projects only) in a timesheet
line without selecting a project first.
@robinkeunen robinkeunen force-pushed the 16.0-mig-coopiteasy_custom branch from 35fee29 to 63cf94c Compare February 1, 2024 16:24
Copy link
Member

@carmenbianca carmenbianca left a comment

Choose a reason for hiding this comment

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

lgtm, one comment

Comment on lines +7 to +17
@openupgrade.migrate()
def migrate(env, version):
env.cr.execute(
"""
alter table project_task drop column if exists reviewer_id;
alter table project_task drop column if exists tester_id;
alter table project_task drop column if exists int_priority;
drop table link_task_relation_table;
"""
)
env.cr.commit()
Copy link
Member

Choose a reason for hiding this comment

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

this is typically not done in oca projects, preferring to keep (dead) data. here we may not care because we're the only user?

Copy link
Member

Choose a reason for hiding this comment

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

yes, i think we can clean it up : these fields were very sparsely used, the data in it is not relevant.

@github-grap-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

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

Successfully merging this pull request may close these issues.

6 participants