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

[MIG] Migration from 9.0 to 10.0 web_access_rule_buttons #628

Merged
merged 2 commits into from
Jun 5, 2017

Conversation

daramousk
Copy link
Member

No description provided.

@daramousk daramousk mentioned this pull request May 17, 2017
59 tasks
@pedrobaeza
Copy link
Member

It would be interesting to rename to singular as stated in the OCA guidelines.

@StefanRijnhart
Copy link
Member

@pedrobaeza Not sure in this case, as it does not refer to a data model but to the standard edit/create/delete buttons in the interface.

@StefanRijnhart StefanRijnhart added this to the 10.0 milestone May 18, 2017
Copy link

@felixvillafranca felixvillafranca left a comment

Choose a reason for hiding this comment

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

It's Ok; but there is a variable unused.

var core = require("web.core");

var FormView = require("web.FormView");
var ListView = require("web.ListView");

Choose a reason for hiding this comment

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

This line could be removed

var ListView = require("web.ListView");
FormView.include({

load_record() {
Copy link
Member

Choose a reason for hiding this comment

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

Sorry friend, we can't allow this syntax until it is supported by PhantomJS, or all JS tests would potentially break. You have to use the old, ugly and safe load_record: function () {... syntax.

load_record() {
var self = this;
return this._super.apply(this, arguments).then(function() {
self.show_hide_buttons()
Copy link
Member

Choose a reason for hiding this comment

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

Feels better IMHO to proxy the function like this._super.apply(this, arguments).then($.proxy(this.show_hide_buttons, this))

show_hide_buttons() {
var self = this;
this.dataset.call("check_access_rule_all", [ [ this.datarecord.id ], [ "write" ] ]).then(function(accesses) {
console.log(accesses)
Copy link
Member

Choose a reason for hiding this comment

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

IMHO this deserves a console.debug instead. Please add semicolon.

if (this.$buttons) {
var button = this.$buttons.find(".o_form_button_edit");
if (button) {
button.prop("disabled", !access);
Copy link
Member

Choose a reason for hiding this comment

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

4 spaces, no tabs.

@daramousk
Copy link
Member Author

@yajo Changes implemented

Copy link

@felixvillafranca felixvillafranca left a comment

Choose a reason for hiding this comment

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

Change Ok.
Funtional Ok.

@yajo yajo merged commit a839651 into OCA:10.0 Jun 5, 2017
@pedrobaeza
Copy link
Member

I would like to have this renamed to singular, but too late... Maybe on v10?

@pedrobaeza
Copy link
Member

Ouch, I see that this is v10. Uf, too bad. Can you maybe make a PR renaming it?

espo-tony pushed a commit to espo-tony/web that referenced this pull request Apr 10, 2019
* [MIG] Migration from 9.0 to 10.0

* Refactoring javascript code
JonathanStein pushed a commit to steingabelgaard/web that referenced this pull request Apr 26, 2020
* [MIG] Migration from 9.0 to 10.0

* Refactoring javascript code
tirix pushed a commit to tirix/web that referenced this pull request Aug 19, 2020
* [MIG] Migration from 9.0 to 10.0

* Refactoring javascript code
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.

5 participants