Review: Needs Fixing
Does not work at all, in either Firefox 5 or Safari 5:
* Click on database button
* DB menu is broken (dark gray underside is wider than the db buttons)
* Error in console:
chrome.js:469 TypeError: 'undefined' is not an object (evaluating
'this.session.rpc')
* Buttons are not active, clicking them does not do anything.
>From a code point of view:
* CSS code indented with tabs. Bad. Spaces only.
== Python ==
* Comments committed in the code
* Why not create a separate method for each db operation? It's not like they
have much (if any) thing in common, and it makes the code far more complex
* Error handling is debatable, why not let non-AccessDenied exceptions bubble
up normally?
* A blanket catch of all exceptions is incorrect, error-handling code should
only ever catch xmlrpclib.Fault (indicating an XML-RPC error) and should
re-raise in case it's not an AccessDenied.
* confirm_password is not even checked in change_password… Neither are most
values (what's going to happen if a user sends a request with an empty
password?)
* Why do you prepend `('en_US', 'English (US)')` to the list? It's already in
the list returned by get_lang. And you should not ignore exceptions the way
it's done in get_lang.
Overall, Python code seems full of redundancies and more complex than required
== JS ==
* Bunch of unused instance variable (this/self.db_string seems unused, so does
this.option_id)
* Invalid markup (center)
* Why this.$option_id.html('')? Why not just call .empty? Also how comes you
have to empty it, what's there to remove?
* Why do you fetch the database list and never use it?
* Why do you fetch the languages list and never use it?
Also, ideally should select the user's (browser) language as default
* Wouldn't using jQuery's serializeArray to get form values be easier than
manually query each field?
* Lots of redundancies in JS code as well, handling of errors is just
copy/pasted from one to the next.
* Not sure notifications are sufficient for db changes performed by these
forms, but I can't test since I can't get it to work (see first part)
== Template ==
Intendation seems incorrect in some places, also uses tabs.
--
https://code.launchpad.net/~openerp-dev/openerp-web/database-noz/+merge/68366
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openerp-web/database-noz.
_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help : https://help.launchpad.net/ListHelp
