Skip to content

[9.0][IMP][base_custom_info] New types and access rules system.#492

Merged
pedrobaeza merged 6 commits into
OCA:9.0from
Tecnativa:9.0-base_custom_info-field_types
May 29, 2017
Merged

[9.0][IMP][base_custom_info] New types and access rules system.#492
pedrobaeza merged 6 commits into
OCA:9.0from
Tecnativa:9.0-base_custom_info-field_types

Conversation

@yajo

@yajo yajo commented Aug 3, 2016

Copy link
Copy Markdown
Member

Now you can define properties types, and access rules are inherited from the model/record linked to the custom info record.

@Tecnativa

@yajo
yajo force-pushed the 9.0-base_custom_info-field_types branch from 2f981d9 to c59de8d Compare August 3, 2016 16:13
@pedrobaeza

Copy link
Copy Markdown
Member

Texts fields should be translatable. Can that be configurable in the definition? I don't think so, but maybe we can make some hack in the method fields_get or fields_view_get.

If not, we should define then the field as always translatable but include anyway a flag that allows to specify this translatable possibility. If you don't mark this flag, then at write time, you have to synchronize by code all the translations to the current one, so it behaves as if the field is not translatable in the background.

@pedrobaeza

Copy link
Copy Markdown
Member

Why the test coverage drop?

To define a template, you need to:

* Go to *Settings > Custom Info > Templates*.
* Go to *Custom Info > Templates*.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this a top level menu?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't possible to put inside another top level menu? (we have enough top level menus already)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I realized that if you have access to setting the templates and values, you should be able to manage them from their menu more comfortably, no need to be an admin for that. Given in v9 the trend is to have more dedicated menus, I'm just following it. Besides, this base module isn't related to any other by default.

I guess the increase of top level menus comes from the enterprise version, where they are beautiful icons in a smartphone-like home panel. After all, the problem is that the backend community interface is way old-fashioned. Nobody will care about them if some day OCA creates a good alternative 😋

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, but after this day comes, we have a lot of them... Anyway, if you don't find any parent menu, let's keep it this way. But we should provide then record rules to avoid one person without product access that modifies custom info profiles.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Forget about the record rule. You have answered me in other comment.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note- mobile ready interface is scheduled in one of our sprints, so coming soon 😄

@yajo

yajo commented Aug 4, 2016

Copy link
Copy Markdown
Member Author

About coverage drop, I think it's because this module had no tests and it just got bigger.

I'd need some model to implement the custom.info mixin to test this. Do you have any idea on how to make the tests?

Can I declare a new model inside a test? Or do I create a transient model just for testing purposes, that does not appear anywhere else? But it would exist even in production databases...

@pedrobaeza

Copy link
Copy Markdown
Member

Make it over res.users model.

@pedrobaeza

Copy link
Copy Markdown
Member

Or even over res.partner. They are both models that are in the base module.

@yajo

yajo commented Aug 4, 2016

Copy link
Copy Markdown
Member Author

Good idea! 💡

@pedrobaeza

Copy link
Copy Markdown
Member

Change spanish translation of the menu "Custom info" to "Inf. personalizada" to shorten the possible the menu width

<field name="name">base.custom.info.value.tree</field>
<field name="model">custom.info.value</field>
<field name="arch" type="xml">
<tree string="Custom Property Values">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Put create="0" delete="0" to avoid modifications on the populated list

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Actually I thought that letting users put additional properties could help under some circumnstances. What do you think?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think so. This will lead to conflicts (lines that doesn't belong to the same model), and it also break the concept of information template. If you need something extra, make another profile. The only problem I see with this that the same property can't be reused for several templates. Or it can be?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not right now. I can add that to known issues.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

BTW, this inability simplifies one algorithm. I'll commit soon.

@yajo
yajo force-pushed the 9.0-base_custom_info-field_types branch from ad15c7e to 9a88696 Compare August 4, 2016 15:43
@yajo

yajo commented Aug 4, 2016

Copy link
Copy Markdown
Member Author

This module should be ready, but there's a bug that me and @pedrobaeza found, that seems to affect v9 onchange methods.

When you choose a template from the partner form, it fills the values, but properties remain empty:

captura de pantalla de 2016-08-04 17-47-37

@yajo

yajo commented Aug 4, 2016

Copy link
Copy Markdown
Member Author

I honestly don't know why, but it seems to work now, after adding custom tree and form views inside the field.

Does anybody know how to disable the button "save & new" in the modal form?

Comment thread base_custom_info/models/custom_info.py Outdated
from openerp.tools.safe_eval import safe_eval


class CustomInfoModelLink(models.AbstractModel):

@lasley lasley Aug 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we get these models in separate files please? Relevant guideline Feel free to ignore me here, my bad didn't realize this wasn't a MIG for a second

@lasley

lasley commented Aug 4, 2016

Copy link
Copy Markdown
Contributor

@yajo does action_buttons="false" in the form tag declaration for the modal work? Admittedly never used it, but I found this code the other day, which seems to elude to what we want.

@yajo

yajo commented Aug 5, 2016

Copy link
Copy Markdown
Member Author

@yajo

yajo commented Aug 5, 2016

Copy link
Copy Markdown
Member Author

This should be ready to merge now.

@lasley

lasley commented Aug 5, 2016

Copy link
Copy Markdown
Contributor

Lame about the hardcoded form attrs. LGTM thanks @yajo 👍

Comment thread base_custom_info/models/custom_info.py Outdated
_name = "custom.info.value"
_rec_name = 'value'
_order = "property_id"
_order = "name"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this should be ordered by a sequence defined in the template (not sortable at user level).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I like that.

@yajo
yajo force-pushed the 9.0-base_custom_info-field_types branch 2 times, most recently from 22d5676 to 736adbc Compare August 8, 2016 09:39
@yajo

yajo commented Aug 8, 2016

Copy link
Copy Markdown
Member Author

I rebased to fix conflicts, and took the time to squash some commits. Sorry reviewers 😅. Notice that the really new 2 commits are the last 2.

@yajo
yajo force-pushed the 9.0-base_custom_info-field_types branch from 11fde67 to a379e16 Compare August 8, 2016 09:52
@yajo

yajo commented Dec 16, 2016

Copy link
Copy Markdown
Member Author

That's because travis failures are from other addons, I rebased now, let's see if some get fixed.

@lasley lasley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @yajo - comments inline

PS big 👍 to the images

Comment thread base_custom_info/models/custom_info.py Outdated
to_add = props_good - props_enabled
to_rm = props_enabled - props_good
# Remove remaining properties
# HACK https://github.com/odoo/odoo/pull/13480

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is merged, good to remove?

Comment thread base_custom_info/models/custom_info.py Outdated
compute="_compute_dirty_templates",
)

# HACK https://github.com/odoo/odoo/pull/11042

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Closed as no-fix. Probably good to remove the comment?

Comment thread base_custom_info/models/custom_info.py Outdated
"dirty_templates").action_custom_info_templates_fill()
return res

# HACK https://github.com/odoo/odoo/pull/11042

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Closed as no-fix. Probably good to remove the comment?

Comment thread base_custom_info/models/custom_info.py Outdated
info_values.unlink()
return res

@api.one

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Deprecated, please switch to multi

@api.multi
def all_custom_info_templates(self):
"""Get all custom info templates involved in these owners."""
return (self.mapped("custom_info_template_id") |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bitwise or, on a recordset?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This works as with normal python sets, joining all results and removing duplicates.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TIL

self.default_value, self.field_type, self)
except ValueError:
selection = dict(
self._fields["field_type"].get_description(self.env)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing a comma between this and next arg. Although why dict() instead of {}?

if "value" in vals:
self.env.context.skip_required = True
result = super(CustomInfoValue, self).create(vals)
# HACK https://github.com/odoo/odoo/pull/11042

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

closed nofix

.action_custom_info_templates_fill()
return result

# HACK https://github.com/odoo/odoo/pull/11042

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

closed nofix

s[s.field_name] = self._transform_value(
s.value, s.field_type, s.property_id)

@api.one

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

deprecated

@api.onchange("property_id")
def _onchange_property_set_default_value(self):
"""Load default value for this property."""
for record in self:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No need to rec loop in an onchange

@pedrobaeza
pedrobaeza force-pushed the 9.0-base_custom_info-field_types branch 2 times, most recently from e87ec4a to 0c4e8a1 Compare February 13, 2017 01:32
value.ensure_one()
value = self.env["custom.info.option"].search([
("property_ids", "in", properties.ids),
("name", "ilike", u"%{}%".format(value)),

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These percents do nothing, right? 🤔 (It's not =ilike)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Uhm, can be, I wasn't sure and put that for assuring.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@pedrobaeza
pedrobaeza force-pushed the 9.0-base_custom_info-field_types branch from 0c4e8a1 to 6c80dea Compare February 17, 2017 11:13
@sergio-teruel
sergio-teruel force-pushed the 9.0-base_custom_info-field_types branch from a2939a0 to 0472dfe Compare April 3, 2017 07:26
yajo and others added 5 commits April 4, 2017 23:05
* Now you can define properties types, and access rules are inherited from the model/record
  linked to the custom info record.
* Simplified version of computed value.
* Implement for res.partner.
* Add tests and fix bugs discovered in the meantime.
* Allow to disable partner custom info tab, and custom info menu.
* All of it can be set within general settings.
* Now, by default, this module does not display custom info for partners unless in demo mode.
  Better fit for a base module.
* You can disable the top menu entry too if it disturbs you, or enable it for everybody.
* Give a special form when editing in partner custom info tab.
* Sortable properties.
* Sort values at onchange time.
* Improve performance in onchange.
* Split in several model files.
- Select an option with an additional template and get it filled on the owner.
- Drop support for onchange, too many bugs to workaround.
- Improve demo data.
- Improve tests.
- Almost cool material icon, this is an app now!
- Fun pics.
@pedrobaeza
pedrobaeza force-pushed the 9.0-base_custom_info-field_types branch from c70bcf2 to 65967a7 Compare April 4, 2017 21:05
@rafaelbn

rafaelbn commented Apr 5, 2017

Copy link
Copy Markdown
Member

@lasley could you review here last changes, your revision is blocking, thanks!
@yajo please check travis, is failing

@yajo

yajo commented Apr 19, 2017

Copy link
Copy Markdown
Member Author

I just tested this and it works like a charm now! I hope I can fix Travis soon & merge, awesome pals.

@dreispt

dreispt commented Apr 20, 2017

Copy link
Copy Markdown
Member

I tried a Travis rebuild 🤞

jesusVMayor pushed a commit to Comunitea/sale-workflow that referenced this pull request Apr 24, 2017
pedrobaeza pushed a commit to Tecnativa/product-variant that referenced this pull request Apr 28, 2017
@yajo

yajo commented May 29, 2017

Copy link
Copy Markdown
Member Author

It should go 💚 now.

@pedrobaeza
pedrobaeza merged commit 50e9a8a into OCA:9.0 May 29, 2017
@yajo
yajo deleted the 9.0-base_custom_info-field_types branch May 30, 2017 08:02
Roodin pushed a commit to Comunitea/product-variant that referenced this pull request Jun 1, 2018
pedrobaeza pushed a commit to Tecnativa/product-variant that referenced this pull request Aug 16, 2018
fuentes73 pushed a commit to Studio73/product-variant that referenced this pull request Oct 23, 2019
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (10.0)
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.

9 participants