-
-
Notifications
You must be signed in to change notification settings - Fork 91
[add] cms_form + cms_form_example #9
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
Conversation
cd7eaa2 to
3d73c7b
Compare
1 similar comment
Few different ways - in your instance prolly the coveragerc file would be best. My warning is to not go excluding things all over the place because it's really just covering up a problem. Anything not tested is a ticking timebomb IMO. That said, we're definitely fine ignoring an example module. |
.coveragerc
Outdated
| @@ -0,0 +1,4 @@ | |||
| [run] | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a look at https://github.com/LasLabs/python-cfssl/blob/master/.coveragerc
You want report section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, the docs do not mention [report] here https://coverage.readthedocs.io/en/coverage-4.2/source.html#execution
anyway, I updated it as per your example, but is still looking for cms_form_example.
Better look at this tomorrow, instead of checking it in the middle of my baby's bath 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked further and it looks like coveragerc isn't an option because the template is being used instead. It would probably be best to just use
# pragma: no cover
near the top of the python files then 😦
|
@tedsalmon please review |
tedsalmon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Awesome work! I have some minor review items, mostly import related stuff that caught my eye :)
cms_form/models/cms_form.py
Outdated
| # Copyright 2017 Simone Orsi | ||
| # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). | ||
|
|
||
| from openerp import models |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little nit picky but these imports are not ordered alphabetically as OCA guidelines ask. Perhaps you'd consider importing in a single line?
from openerp import _, exceptions, models
| from ..utils import DEFAULT_LOADERS, DEFAULT_EXTRACTORS | ||
| from ..import widgets | ||
|
|
||
| import inspect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These imports should be above the Odoo import
cms_form/models/cms_form_mixin.py
Outdated
| value = extractor(self, fname, value, **request_values) | ||
| if value is None: | ||
| # we assume we do not want to override the field value. | ||
| # a tipical example is an image field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/tipical/typical
cms_form/models/cms_search_form.py
Outdated
| def form_search_domain(self, search_values): | ||
| """Build search domain. | ||
|
|
||
| TODO... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's TODO here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well: explain how we build domain and maybe improve it a little bit.
cms_form/models/test_models.py
Outdated
| # Copyright 2017 Simone Orsi | ||
| # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). | ||
|
|
||
| from openerp import models |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alphabetical import order, again :)
cms_form/tests/__init__.py
Outdated
| from . import test_form_search | ||
| from . import test_form_render | ||
| from . import test_loaders | ||
| from . import test_controllers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import alphabetically :)
cms_form/tests/test_controllers.py
Outdated
| from .common import fake_request | ||
| from .common import FormHttpTestCase | ||
| from ..controllers import main | ||
| import mock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be above your local imports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tedsalmon tnx! I'll go trough them.
1ebda08 to
26ed9c4
Compare
1 similar comment
guewen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow
cms_form/README.rst
Outdated
| if extra_values.get('notify_partner'): | ||
| # do what you want here... | ||
|
|
||
| ``notifiy_partner`` will be included into the form but it will be discarded on create and write. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/notifiy/notify/g
7dc324b to
a84f330
Compare
a84f330 to
3546f26
Compare
1 similar comment
|
@simahawk - Did you have any plans to up the test coverage? |
|
@simahawk - I set this to WIP because commits are still being added. Let us know when it's ready for review again. |
|
@lasley yes pls :) I'm still cleaning/fixing/adding small improvements and I'll move widgets to models. |
|
BTW I'll re-license already merged modules in a separated PR |
|
@pedrobaeza would you mind hit the button here? I'll improve test cov later, probably tomorrow. |
CMS Form
Basic website contents form framework. Allows to define front-end forms for every models in a simple way.
If you are tired of re-defining every time an edit form or a search form for your odoo website,
this is the module you are looking for.
Features
automatic form generation (create, write, search)
automatic route generation (create, write, search)
automatic machinery based on fields' type:
highly customizable
works with every odoo model
works also without any model
add handy attributes to models inheriting from
website.published.mixin:cms_add_url: lead to create form view. By default/cms/form/create/my.modelcms_edit_url: lead to edit form view. By default/cms/form/edit/my.model/model_idcms_search_url: lead to search form view. By default/cms/form/search/my.model(extracted from official README)