-
-
Notifications
You must be signed in to change notification settings - Fork 181
[19.0][ADD] sale_portal_debranding: Remove Odoo branding from sale portal page #120
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
base: 19.0
Are you sure you want to change the base?
Conversation
cb959a8 to
2067a19
Compare
|
Hello @pedrobaeza, I'am not sure if I did anything wrong here and how I can progress from here. Maybe you can give me a hint? Have a nice evening, Jappi00 |
|
Hello @jappi00 it seems correct. Now, you have to find reviewers https://odoo-community.org/page/review You can review other PRs, and ask in exchange that they review yours. |
ljmnoonan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work!
Honestly, I do think this should be a part of portal_debranding as it is already doing stuff similar to what this module does, so I would suggest opening a new PR as such.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I highly recommend a more thorough description of where exactly this modal is, and possibly a screenshot as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tuned the description a little bit.
Was my first idea too. But that module do not depend on sale. We would take away the possibility to use the portal debranding module for users which only use purchase and not the sale module. |
|
And @ljmnoonan, thanks for the review. I appreciate it! I’ll update it shortly. |
Ahh, I see. I wonder if there is a way to make a view "optional" and not crash everything if it does not find its parent. I'll have to look into it |
9d25f99 to
a7deb37
Compare
I don't think so but it would be a good thing since we have the exact same modal on purchase and this would mean we would need one more module. Maybe we could autoinstall those modules with the portal debranding? |
bc4a7b8 to
3fe02b5
Compare
|
This module should be an extra one, as the base one can't depend on sale. |
3fe02b5 to
6b9c85a
Compare
|
@ljmnoonan Sorry, I strugeled a little bit with pre-commit. I hope I solved everything. Not sure if the more defined xpath / view is good. Was a little bit hard to find a good way for targeting the right elements. |
ljmnoonan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functional test is good!
A few nitpicks which you can ignore if you want :)
650bf0b to
2d0b2d6
Compare
2d0b2d6 to
3512889
Compare
Fixed those. 🫡 |
Created my first module for this issue #115 . I tried to follow all guidelines and I hope I did everything right.
What it does?
Replace all references, except to the docs for integrations, from the "Connect youre System" Modal on the sale portal page