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

[FIX] web: click on home menu on next tick #31949

Closed
wants to merge 1 commit into from

Conversation

d-fence
Copy link
Contributor

@d-fence d-fence commented Mar 19, 2019

Since odoo/enterprise@b830ea7ed2a the enterprise home menu is asynchronous.
Because of that, the clientActionCount variable is updated too late and
this counter is screwed up, making the test fail.

With this commit a setTimeout 0 is used to click on home menu in the
next tick and then update the current action count.

Also

  • the test is adapted to stop immediately when an error is catched.
  • the initial click on home menu is moved into debug manager because
    the python test launcher user the correct location already.

Thanks to @VincentSchippefilt and @aab-odoo

@d-fence d-fence requested a review from aab-odoo March 19, 2019 15:23
@C3POdoo C3POdoo added the RD research & development, internal work label Mar 19, 2019
@@ -24,7 +24,7 @@
AbstractController.include({
start: function(){
this.$el.attr('data-view-type', this.viewType);
return this._super();
return this._super(arguments);
Copy link
Contributor

Choose a reason for hiding this comment

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

this._super.apply(this, arguments);

@@ -36,7 +36,7 @@
if (Discuss) {
Discuss.include({
_fetchAndRenderThread: function() {
return this._super().then(function (){
return this._super(arguments).then(function (){
Copy link
Contributor

Choose a reason for hiding this comment

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

this._super.apply(this, arguments);

setTimeout(function(){
var $homeMenu = $("nav.o_main_navbar > a.o_menu_toggle.fa-th");
$homeMenu.click();
}, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this really work?? This is not what we did earlier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it works.
The previous thing was to wait for ".o_home_menu_background" with an interval

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid it wasn't. We clicked directly on the button, and then made a Promise that resolved when the home menu was in the DOM, and we returned that promise, making the whole thing wait for it to be displayed before clicking on the next app.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this works, it's by chance, and we are still clicking on something that is not in the DOM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I revert with the previous code then.

Copy link
Contributor

@aab-odoo aab-odoo Mar 21, 2019

Choose a reason for hiding this comment

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

$homeMenu.click();
return new Promise(function (resolve) {
    setTimeout(resolve, 0);
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I see, I thought that searching for the home menu in the setTimeout would be sufficient to have it in the DOM

@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Mar 21, 2019
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Mar 21, 2019
Since odoo/enterprise@b830ea7ed2a, the enterprise home menu is asynchrnonous.
Because of that, the clientActionCount variable is updated too late and
this counter is screwed up, making the test fail.

With this commit a setTimeout 0 is used to click on home menu in the
next tick and then update the current action count.

Also
 * the test is adapted to stop immediately when an error is catched.
 * the initial click on home menu is moved into debug manager because
   the python test launcher uses the correct location already.

Thanks to @VincentSchippefilt and @aab-odoo
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Mar 21, 2019
@d-fence
Copy link
Contributor Author

d-fence commented Mar 21, 2019

@robodoo r+

robodoo pushed a commit that referenced this pull request Mar 21, 2019
Since odoo/enterprise@b830ea7ed2a, the enterprise home menu is asynchrnonous.
Because of that, the clientActionCount variable is updated too late and
this counter is screwed up, making the test fail.

With this commit a setTimeout 0 is used to click on home menu in the
next tick and then update the current action count.

Also
 * the test is adapted to stop immediately when an error is catched.
 * the initial click on home menu is moved into debug manager because
   the python test launcher uses the correct location already.

Thanks to @VincentSchippefilt and @aab-odoo

closes #31949

Signed-off-by: Christophe Monniez (moc) <moc@odoo.com>
@robodoo
Copy link
Contributor

robodoo commented Mar 21, 2019

Merged, thanks!

@robodoo robodoo closed this Mar 21, 2019
@fw-bot fw-bot deleted the master-fix-click-all-moc branch October 19, 2019 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI 🤖 Robodoo has seen passing statuses RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants