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_create_write_confirm: fix JS, usage. Minor refactoring. #2576

Merged

Conversation

ilyasProgrammer
Copy link
Member

No description provided.

@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-web_create_write_confirm-fixes branch from afe8f18 to 1dd067b Compare July 27, 2023 10:48
@ilyasProgrammer
Copy link
Member Author

@simahawk could you please review ? You refactoring changes are here. Also some small JS fixes.

@francesco-ooops
Copy link

@tarteo fyi :)

@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-web_create_write_confirm-fixes branch from 1dd067b to 841db55 Compare August 14, 2023 08:41
@francesco-ooops
Copy link

@simahawk ping :)

@francesco-ooops
Copy link

@simahawk in this PR you can find the changes you requested here: #2487 (review)

also without this fix a few features in runboat are blocked by this module, so a merge would be very welcome :)

@francesco-ooops
Copy link

@yajo what do you think? :)

@yajo
Copy link
Member

yajo commented Sep 20, 2023

Sorry, I don't know this module and I don't know what this is trying to fix, so I'm not the best one to review here.

@francesco-ooops
Copy link

@OCA/web-maintainers some runboat features are blocked since this module is displaying errors. This PR fixes it. Can we merge please?

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

functional review on runboat. Something looks broken.
step to reproduce :

  • go to settings / users > create new user

  • click on save.

Error:


Error: event is undefined
ev@http://oca-web-14-0-pr2576-841db5557087.runboat.odoo-community.org/web_create_write_confirm/static/src/js/pop_up_confirmation.js:100:13
display_popup/<@http://oca-web-14-0-pr2576-841db5557087.runboat.odoo-community.org/web_create_write_confirm/static/src/js/pop_up_confirmation.js:93:26
display_popup@http://oca-web-14-0-pr2576-841db5557087.runboat.odoo-community.org/web_create_write_confirm/static/src/js/pop_up_confirmation.js:44:13
_onSave/<@http://oca-web-14-0-pr2576-841db5557087.runboat.odoo-community.org/web_create_write_confirm/static/src/js/pop_up_confirmation.js:29:22```

@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-web_create_write_confirm-fixes branch from 5a45a7e to a9f2243 Compare October 2, 2023 08:03
@ilyasProgrammer
Copy link
Member Author

updated

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Functional review. LGTM.

No Code review.

@francesco-ooops
Copy link

@simahawk when you can. Thanks!

@francesco-ooops
Copy link

@simahawk what do you say?

Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

LG

@simahawk
Copy link
Contributor

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-2576-by-simahawk-bump-minor, awaiting test results.

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

@OCA-git-bot OCA-git-bot merged commit ca2e322 into OCA:14.0 Oct 18, 2023
8 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at df3a5cc. 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants