MIG 9.0 support_branding

[angelmoya]

No description provided.

[hbrunn]

you miss commits from 8.0 - follow https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-9.0 to have them in your PR

[hbrunn]

something's missing here

[hbrunn]

after you rewrote the js part to v9 api, I'd consider it a serious contribution. As it is, I think this is not warranted

[hbrunn]

rewrite to v9 api

[angelmoya]

@hbrunn thanks for yout comments I will rewrite

[angelmoya]

@hbrunn I rewrite js, please check it

[hbrunn]

on runbot, go to the app store, this will trigger an exception. But the exception popup doesn't have the send button, nor the text field to fill in stuff. So it seems you'll have to adapt something there too

[hbrunn]

where do you require Model? I think you should require web.Model

[angelmoya]

It was fixed

[angelmoya]

You need to set support email on parameters to get this form and send
button.
I will fix this js

El 29/9/2016 19:33, "Holger Brunn" notifications@github.com escribió:

@hbrunn requested changes on this pull request.

on runbot, go to the app store, this will trigger an exception. But the
exception popup doesn't have the send button, nor the text field to fill in

stuff. So it seems you'll have to adapt something there too

In support_branding/static/src/js/support_branding.js
#430 (review):

         var self = this,

•            ir_config_parameter = new openerp.web.Model('ir.config_parameter');
  

•            ir_config_parameter = new Model('ir.config_parameter');
  

where do you require Model? I think you should require web.Model

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#430 (review), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGcAhVqD1q_DM_8P2PScXqPbTl3_-1jTks5qu_ZcgaJpZM4KJnOV
.

[hbrunn]

I just filled in an email address, but still no send button on the exception

[angelmoya]

I test... send buttons is showed and open system app to send email... but you need to scroll to see that

[angelmoya]

OK, tested on Runbot with module test-exceptions....

It's works nice with Undefined type error, not with other error types, I will check it

[angelmoya]

Finally I tested with test-exceptions, and is working well.

It show email form on ERROR and not show on WARNING.

I can do also to show form on warning, but I think that email form is needed for ERROR and not for WARNING, because warning could be use to show information to user.

This is the xml that is extended to show email form:

https://github.com/OCA/OCB/blob/9.0/addons/web/static/src/xml/base.xml#L31

And this is the record for the warning:

https://github.com/OCA/OCB/blob/9.0/addons/web/static/src/xml/base.xml#L22

If it's needed to show email form on warning we only need to add to this record.

I think that is not needed for warning, what do you think @hbrunn ?

[pedrobaeza]

@angelmoya Where's the updated code?

[angelmoya]

I did not update code, it was right, I only review functionality

El 14/10/2016 19:17, "Pedro M. Baeza" notifications@github.com escribió:

@angelmoya https://github.com/angelmoya Where's the update code?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#430 (comment), or mute the
thread
https://github.com/notifications/unsubscribe-auth/AGcAhZLKzmztb8kAV-hmIvgzxF0DZW44ks5qz7kRgaJpZM4KJnOV
.

[pedrobaeza]

OK, let's see what @hbrunn says

[hbrunn]

I just made @angelmoya's homework and debugged through this, finding that this cannot ever work because of https://github.com/angelmoya/web/blob/d0865bbfa1d0fb79604837ee7db13b567840ecd4/support_branding/static/src/js/support_branding.js#L10 - extend returns a reference to the new object, but doesn't touch the original object. What you want here is include. Do you actually test the code you write?

And then there's still #430 (comment) pending

[hbrunn]

superseded by #469