> 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
I have fixed the code, It was because of merging the main branch in to my
branch.
You can check this now.
>
> From a code point of view:
>
> * CSS code indented with tabs. Bad. Spaces only.
Changed.
>
> == Python ==
> * Comments committed in the code
- I Committed manually. Which will be going to be implemented.
> * 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
- There is not any reason to create all together.
- I Separated it.
> * Error handling is debatable, why not let non-AccessDenied exceptions bubble
> up normally?
- I changed little bit.
> * 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.
- I have tried to follow the exception handling like 6.0.
> * 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?)
- I used jQuery.validate.js for this. This will work perfectly.
> * 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.
>
- Prepending the en_US in the list is to have the default English on top.
Its same we followed in 6.0 code.
- Raised error.
> 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)
- Removed self.db_string, and added static header on xml, and this.option_id is
necessary which contain operation area.
> * Invalid markup (center)
- removed, actually without any tag also, its fine.
> * Why this.$option_id.html('')? Why not just call .empty? Also how comes you
> have to empty it, what's there to remove?
- Suppose I select drop database option, and i go to logout and again i come to
database then drop database option will get selected, so i want it to be
cleared but now its of no use for the moment.
> * Why do you fetch the database list and never use it?
> * Why do you fetch the languages list and never use it?
- I have used both once rendering the page. In base.xml file in drop database,
db list is coming from here only, I passed (QWeb.render("DropDB", self))
> 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?
- Its good options, I didnt use it before. Now its done.
> * 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)
- You can check and let me know if more correction.
>
> == Template ==
> Intendation seems incorrect in some places, also uses tabs.
- Almost solved with spaces.
--
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
