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.

Sign up for GitHub

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

Jump to bottom

[17.0][MIG] crm_stage_probability: Migration to 17.0 #549

Merged
merged 10 commits into from
May 14, 2024
Merged

[17.0][MIG] crm_stage_probability: Migration to 17.0 #549

merged 10 commits into from
May 14, 2024

Conversation

MohamedOsman7
Copy link

No description provided.

@MohamedOsman7 MohamedOsman7 marked this pull request as draft February 29, 2024 17:13
CRogos
CRogos reviewed Mar 4, 2024
View reviewed changes
Copy link

@CRogos CRogos left a comment

Choose a reason for hiding this comment

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

I think we can squash these commits into the migration 14.0 commit:
image

crm_stage_probability/views/crm_lead.xml Outdated
@@ -7,14 +7,14 @@
<field name="arch" type="xml">
<xpath
expr="//button[@name='action_set_automated_probability']"
position="before"
position="after"
Copy link

Choose a reason for hiding this comment

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

I think this should stay before and should look like this
image

crm_stage_probability/views/crm_lead.xml Outdated
@@ -7,14 +7,14 @@
<field name="arch" type="xml">
<xpath
expr="//button[@name='action_set_automated_probability']"
position="before"
position="after"
>
<field name="is_stage_probability" invisible="1" />
<button
class="ps-0 ps-md-2 btn btn-link"
Copy link

Choose a reason for hiding this comment

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

Could you check if this class fixes the layout.
class="btn d-inline-block px-2 py-0 btn-link"

@MohamedOsman7 MohamedOsman7 force-pushed the 17.0-mig-crm_stage_probability branch from 48c0635 to b00cb27 Compare March 5, 2024 21:52
@MohamedOsman7 MohamedOsman7 marked this pull request as ready for review March 5, 2024 23:18
@MohamedOsman7 MohamedOsman7 requested a review from CRogos March 5, 2024 23:18
@CRogos CRogos mentioned this pull request Mar 28, 2024
Open
20 tasks
@MohamedOsman7 MohamedOsman7 reopened this May 7, 2024
@MohamedOsman7
Copy link
Author

@remi-filament could you please review?

@MohamedOsman7 MohamedOsman7 force-pushed the 17.0-mig-crm_stage_probability branch from b00cb27 to 83a7a83 Compare May 13, 2024 13:16
remi-filament
remi-filament suggested changes May 14, 2024
View reviewed changes
Copy link

@remi-filament remi-filament left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi @MohamedOsman7 thank you for porting this module.
I get a warning when installing :
WARNING odoo17 py.warnings: /opt/odoo/addons/base/models/ir_module.py:178: DeprecationWarning: XML declarations in HTML module descriptions are deprecated since Odoo 17, crm_stage_probability can just have a UTF8 description with not need for a declaration.
I think you need to remove the xml tags (first line of index.html)

remi-filament
remi-filament suggested changes May 14, 2024
View reviewed changes
crm_stage_probability/static/description/index.html Outdated
@@ -0,0 +1,440 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link

Choose a reason for hiding this comment

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

Suggested change
<?xml version="1.0" encoding="utf-8"?>

grindtildeath and others added 10 commits May 14, 2024 14:12
@MohamedOsman7 MohamedOsman7 force-pushed the 17.0-mig-crm_stage_probability branch from 83a7a83 to fce6c90 Compare May 14, 2024 14:17
@MohamedOsman7
Copy link
Author

@remi-filament done

remi-filament
remi-filament approved these changes May 14, 2024
View reviewed changes
Copy link

@remi-filament remi-filament left a comment

Choose a reason for hiding this comment

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

Thanks @MohamedOsman7 LGTM

@OCA-git-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). 🤖

@pedrobaeza
Copy link
Member

/ocabot migration crm_stage_probability
/ocabot merge nobump

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone May 14, 2024
@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-549-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Contributor

@pedrobaeza The merge process could not be finalized, because command git push origin 17.0-ocabot-merge-pr-549-by-pedrobaeza-bump-nobump:17.0 failed with output:

To https://github.com/OCA/crm
 ! [remote rejected]   17.0-ocabot-merge-pr-549-by-pedrobaeza-bump-nobump -> 17.0 (cannot lock ref 'refs/heads/17.0': is at d7dff132c5de8d40f60e8e88c36a60776dbe3ed5 but expected 0aca72675890497599dcd3db4d269fc8110e3eeb)
error: failed to push some refs to 'https://***@github.com/OCA/crm'

@OCA-git-bot OCA-git-bot merged commit 4f54961 into OCA:17.0 May 14, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at d7dff13. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@remi-filament remi-filament remi-filament approved these changes

@CRogos CRogos CRogos approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
17.0
Development

Successfully merging this pull request may close these issues.