Repository navigation
Conversation
|
All commit authors signed the Contributor License Agreement. |
guewen
left a comment
guewen
left a comment
There was a problem hiding this comment.
Thanks! That's right it allows to create channels with with no parent_id field in the xml and then handle it from the UI.
I wonder why the changes in create/write are needed though, and why we can't only rely on the field's default value?
| if "parent_id" in values: | ||
| values = self._with_default_parent(values) |
There was a problem hiding this comment.
If we write with "parent_id": False, I think it should just fail and not replace Falsewith the root channel IMO, what is the use case for this corner case?
There was a problem hiding this comment.
No use case, I just wanted to make this as robust as before and prevent any path to set a channel without parent.
I agree that keeping the failure is cleaner though, I have removed this part.
| if channel.name == "root" and ( | ||
| "parent_id" in values | ||
| or ("name" in values and not self.env.context.get("install_mode")) |
There was a problem hiding this comment.
Why did you need to change the condition?
There was a problem hiding this comment.
Again for robustness against a weird corner case, but this is outdated.
I think the new implementation is cleaner, thank you for your review.
| vals_list = [ | ||
| vals if vals.get("name") == "root" else self._with_default_parent(vals) | ||
| for vals in vals_list | ||
| ] |
There was a problem hiding this comment.
Isn't the default value enough? I do not see why we would pass an empty channel_id to create?
There was a problem hiding this comment.
You are right. We still need to do something to be able to find an existing channel if there is one, see the new version.
17a6d65 to
5f5b5bc
Compare
5f5b5bc to
0b7bb86
Compare
The parent is still enforced on every channels except root, but instead of throwing an error when empty, set it to 'root' by default.
No need to force this in the data anymore, 'root' is the default value and it should stay editable in the DB.
0b7bb86 to
dff164f
Compare
OCA-git-bot
added
mod:queue_job_batch
Channels are configurable at DB level since #962.
To benefit fully from this new feature, we'd like to create channels in the xml data and configure them from the UI.
However, up to now, it is not allowed to create a channel without setting its parent.
I propose to remove this constraint and set 'root' as the default parent. We still make sure that every channel (except root) has a parent, but xml data can now be added without parent_id, and the parent_id won't be overwritten at each update.