Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[MIG] web_m2x_options: Migration to 17.0 #2847

Closed
wants to merge 66 commits into from
Closed

[MIG] web_m2x_options: Migration to 17.0 #2847

wants to merge 66 commits into from

Conversation

dyngnijs
Copy link

No description provided.

@dyngnijs
Copy link
Author

dyngnijs commented Jun 13, 2024 •
edited
Loading

Base functionality should work. Widgets still need to be tested and fine-tuned. For example widget res_partner_many2one gives no error and works with the system parameters but the view options do not work yet.

Update: res_partner_many2one should work as well now.

@rven rven mentioned this pull request Jun 18, 2024
Open
27 tasks
CRogos
CRogos reviewed Jun 18, 2024
View reviewed changes
Copy link
Contributor

@CRogos CRogos left a comment

Choose a reason for hiding this comment

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

Test... limit, create, create and edit working fine.

The commit history is quite long and could be shortened. Here are some examples:
https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate
image

web_m2x_options/readme/USAGE.md Outdated

``` xml
...
<field name="partner_id" options="{'limit': 10, 'create': false, 'create_edit': false, 'search_more': true, 'field_color':'state', 'colors':{'active':'green'}}"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you update the sample because 'state' is not valid anymore.

Copy link
Author

Choose a reason for hiding this comment

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

I have updated it with a working example.

@CRogos CRogos mentioned this pull request Jun 18, 2024
Closed
@mohs8421
Copy link

I started this concurring migration here: #2848
From what I saw, your branch is ahead in regards to compatibility I assume, however, I think there are two parts in my attempt, which you might want to take over into this one:

  • I refactored the relational_utils.esm.js, to reduce it's complexity, however that might lack some testing, so look at it with some caution.
  • The cross-dependency with the mass_mailing module has been extracted into a connection module.

Please feel free to take from my PR what you consider useful.

@dynapps dynapps force-pushed the 17.0-mig-web_m2x_options branch 2 times, most recently from dbb0775 to add4730 Compare June 19, 2024 09:34
@dyngnijs
Copy link
Author

Test... limit, create, create and edit working fine.

The commit history is quite long and could be shortened. Here are some examples: https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate image

I have made some squashes. Should commits like this be squashed as well?
image

@dyngnijs
Copy link
Author

I started this concurring migration here: #2848 From what I saw, your branch is ahead in regards to compatibility I assume, however, I think there are two parts in my attempt, which you might want to take over into this one:

* I refactored the relational_utils.esm.js, to reduce it's complexity, however that might lack some testing, so look at it with some caution.

* The cross-dependency with the mass_mailing module has been extracted into a connection module.

Please feel free to take from my PR what you consider useful.

Thanks, I will try to go through it this week.

@CRogos
Copy link
Contributor

CRogos commented Jun 19, 2024

Test... limit, create, create and edit working fine.
The commit history is quite long and could be shortened. Here are some examples: https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate image

I have made some squashes. Should commits like this be squashed as well? image

yes, everything from author oca-*, weblate and when there are multiple translation commits of the same person and language.

git config --global --add rebase.instructionFormat "(%an <%ae>) %s"
git rebase -i origin/17.0

@dynapps dynapps force-pushed the 17.0-mig-web_m2x_options branch 2 times, most recently from dc52f64 to 7627e92 Compare June 21, 2024 12:16
@HaraldPanten
Copy link

@pedrobaeza Could you assign this PR to the migration issue? And I think there is a "forgotten" to be closed --> #2661 (comment)

THX!

@pedrobaeza pedrobaeza mentioned this pull request Jun 25, 2024
Closed
@pedrobaeza
Copy link
Member

/ocabot migration web_m2x_options

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Jun 25, 2024
manuelregidor
manuelregidor suggested changes Jun 25, 2024
View reviewed changes
web_m2x_options/static/src/components/form.esm.js Outdated
event.preventDefault();
event.stopPropagation();
const action = await self.orm.call(
self.props.relation,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be replaced by record.resModel

Copy link
Author

Choose a reason for hiding this comment

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

Correct, the function was broken. It should be fixed now.

web_m2x_options/static/src/components/form.esm.js Outdated
self.actionService.doAction(action);
} else {
const view_id = await self.orm.call(
self.props.relation,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be replaced by record.resModel

Copy link
Author

Choose a reason for hiding this comment

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

Correct, the function was broken. It should be fixed now.

web_m2x_options/static/src/components/form.esm.js Outdated
);
var can_write = self.props.canWrite;
self.dialog.add(FormViewDialog, {
resModel: self.props.relation,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be replaced by resModel: record.resModel

Copy link
Author

Choose a reason for hiding this comment

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

Correct, the function was broken. It should be fixed now.

web_m2x_options/static/src/components/form.esm.js Outdated
);

const write_access = await self.orm.call(
self.props.relation,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be replaced by record.resModel

Copy link
Author

Choose a reason for hiding this comment

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

Correct, the function was broken. It should be fixed now.

web_m2x_options/static/src/components/form.esm.js Outdated
var self = this;
if (self.props.open) {
var context = self.context;
var id = record.data.id;
Copy link
Contributor

Choose a reason for hiding this comment

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

If think is should be replaced by var id = record.resId;

Copy link
Author

Choose a reason for hiding this comment

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

Correct, the function was broken. It should be fixed now.

web_m2x_options/readme/USAGE.md

> Whether to display "Create and Edit..." entry in dropdown panel

`m2o_dialog` *boolean* (Default: depends if user have create rights)
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't make this option work. Are we sure it is working?

web_m2x_options/readme/USAGE.md

> Used to force disable/enable search more button.

`field_color` *string*
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it color_field instead of field_color?

Copy link
Author

@dyngnijs dyngnijs Jun 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

field_color should work using a xml definition like this:
<field name="partner_id" options="{'limit': 10, 'create': false, 'create_edit': false, 'search_more': true, 'field_color':'type', 'colors':{'contact':'green', 'invoice': 'red', 'delivery': 'blue'}}"/>

web_m2x_options/readme/USAGE.md
> A dictionary to link field value with a HTML color. This option has to
> be used with field_color.

`no_open_edit` *boolean* (Default: value of `no_open` which is `False`
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't make this option work. Are we sure it is working?

Copy link
Author

Choose a reason for hiding this comment

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

There is no logic for this option in the v16 code. It is only referenced in the readme/translation files. Not sure what to do with this. Remove it from the readme or look at older versions of this module to see how it worked?

web_m2x_options/readme/USAGE.md
> Makes many2many_tags and one2many rows buttons that open the linked
> resource

`no_color_picker` *boolean* (Default: `False`)
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't make this option work. Are we sure it is working?

Copy link
Author

@dyngnijs dyngnijs Jun 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There is no logic for this option in the v16 code. It is only referenced in the readme files. Not sure what to do with this. Remove it from the readme or look at older versions of this module to see how it worked?

@mohs8421
Copy link

I still get this kind of error, when this module is enabled:

Invalid props for component 'ListMany2ManyTagsAvatarUserField': unknown key 'open', unknown key 'canWrite', unknown key 'nodeOptions'

I'm not sure where it is exactly from, but it does not happen, when the module is not loaded.
The trace points to some template, but it does not resolve into a readable cause.

@dyngnijs
Copy link
Author

I still get this kind of error, when this module is enabled:

Invalid props for component 'ListMany2ManyTagsAvatarUserField': unknown key 'open', unknown key 'canWrite', unknown key 'nodeOptions'

I'm not sure where it is exactly from, but it does not happen, when the module is not loaded. The trace points to some template, but it does not resolve into a readable cause.

Hi,

In which view or action do you get the error?

@dynapps dynapps force-pushed the 17.0-mig-web_m2x_options branch 2 times, most recently from aa85595 to b6928d8 Compare June 28, 2024 13:37
@mohs8421
Copy link

I still get this kind of error, when this module is enabled:

Invalid props for component 'ListMany2ManyTagsAvatarUserField': unknown key 'open', unknown key 'canWrite', unknown key 'nodeOptions'

I'm not sure where it is exactly from, but it does not happen, when the module is not loaded. The trace points to some template, but it does not resolve into a readable cause.

Hi,

In which view or action do you get the error?

It was not really consistent, there had been occurrences where it was not happening on the same pages. The most common thing about this is, that the pages have forms on them, like the settings, or creating a manufacturing order.
Maybe it was related to displaying user avatars in comments or something like that?

CRogos
CRogos suggested changes Jul 2, 2024
View reviewed changes
Copy link
Contributor

@CRogos CRogos left a comment

Choose a reason for hiding this comment

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

Could you also remove some of the cli-bot commits?

pandoc-3.2-1-amd64.deb Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the pandoc-3.2.1-amd64.deb file.

Copy link
Author

Choose a reason for hiding this comment

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

File is removed. For the cli-bot commits, for example these ones:
image

Can I just squash them with the previous commit? Or should I leave those ones alone. The bot squashing is still pretty new to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate

you can squash them with the previous commit.

Copy link
Author

Choose a reason for hiding this comment

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

It should be okay now

remihb
remihb reviewed Jul 17, 2024
View reviewed changes
web_m2x_options/static/description/index.html Outdated
@@ -0,0 +1,412 @@
<?xml version="1.0" encoding="utf-8" ?>
Copy link

@remihb remihb Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can you remove this line please ?

It raises a warning at module installation
DeprecationWarning: XML declarations in HTML module descriptions are deprecated since Odoo 17, web_m2x_options can just have a UTF8 description with not need for a declaration.

Copy link
Author

Choose a reason for hiding this comment

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

I have deleted the line

Nicolas JEUDY and others added 4 commits July 22, 2024 11:30
@dyngnijs
- Rename web_m2o_enhanced to web_m2x_options
3bbb640 
- Add static/description/index.html

fix: use include instead of extend in js function inheritance.

fix: not overwriting the existing object references with the result of the include

fix: update name according to new module name.

fix: error when displaying many2many field without options defined.
@hbrunn @dyngnijs
move all addons to __unported__, set all addons as not installable
cc09eb6
@dyngnijs
fix: obscur wrong path on last commit. recreate portage from scratch.
a7834c0
@hbrunn @dyngnijs
support 'open' on many2many_tags
3f68f84 
[ADD] support 'no_open_edit' on many2one
[FIX] typos
nguyenhk and others added 16 commits July 22, 2024 11:41
@dyngnijs
[FIX] web_m2x_options: fix test_ir_config_parameter
1e84007
@CRogos @dyngnijs
[15.0][FIX] Remove tests from init
8d8446a
@anothingguy @dyngnijs
[16.0][MIG] web_m2x_options: Migration to 16.0
b13b099
@weblate @dyngnijs
Update translation files
35a5e44 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: web-16.0/web-16.0-web_m2x_options
Translate-URL: https://translation.odoo-community.org/projects/web-16-0/web-16-0-web_m2x_options/
@kikopeiro @dyngnijs
Added translation using Weblate (Spanish (Bolivia))
901d1e5
@NICO-SOLUTIONS @dyngnijs
Translated using Weblate (German)
9e80ab6 
Currently translated at 100.0% (14 of 14 strings)

Translation: web-16.0/web-16.0-web_m2x_options
Translate-URL: https://translation.odoo-community.org/projects/web-16-0/web-16-0-web_m2x_options/de/
@nguyenminhchien @dyngnijs
[FIX] web_m2x_options: Avoid constant RPC calls
a0cf73a 
Instead of calling interactively to the server to get the options, they are stored at the beginning of the session.
@dyngnijs
[UPD] Update web_m2x_options.pot
1b5b87c
@weblate @dyngnijs
Update translation files
cde8956 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: web-16.0/web-16.0-web_m2x_options
Translate-URL: https://translation.odoo-community.org/projects/web-16-0/web-16-0-web_m2x_options/
@Ivorra78 @dyngnijs
Translated using Weblate (Spanish)
8707238 
Currently translated at 100.0% (15 of 15 strings)

Translation: web-16.0/web-16.0-web_m2x_options
Translate-URL: https://translation.odoo-community.org/projects/web-16-0/web-16-0-web_m2x_options/es/
@fmdl @dyngnijs
[16.0] web_m2x_options: impossible to open a mass mailing
90fae62 
JS Issue:
OwlError: Invalid props for component 'FieldMany2OneMailingFilter': unknown key 'searchMore', unknown key 'nodeOptions'
    at Object.validateProps (https://odoo.simplyfeu.com/web/assets/1913698-b61fd5b/web.assets_common.min.js:1698:67)
    at Field.template (eval at compile (https://odoo.simplyfeu.com/web/assets/1913698-b61fd5b/web.assets_common.min.js:2057:374), <anonymous>:17:13)
    at Fiber._render (https://odoo.simplyfeu.com/web/assets/1913698-b61fd5b/web.assets_common.min.js:1496:96)
    at Fiber.render (https://odoo.simplyfeu.com/web/assets/1913698-b61fd5b/web.assets_common.min.js:1495:6)
    at ComponentNode.initiateRender (https://odoo.simplyfeu.com/web/assets/1913698-b61fd5b/web.assets_common.min.js:1563:47)
@mymage @dyngnijs
Translated using Weblate (Italian)
2fb5973 
Currently translated at 100.0% (15 of 15 strings)

Translation: web-16.0/web-16.0-web_m2x_options
Translate-URL: https://translation.odoo-community.org/projects/web-16-0/web-16-0-web_m2x_options/it/
@ThomasBinsfeld @dyngnijs
Translated using Weblate (French)
c8938b3 
Currently translated at 40.0% (6 of 15 strings)

Translation: web-16.0/web-16.0-web_m2x_options
Translate-URL: https://translation.odoo-community.org/projects/web-16-0/web-16-0-web_m2x_options/fr/
@glitchov @dyngnijs
Translated using Weblate (French)
88a8519 
Currently translated at 53.3% (8 of 15 strings)

Translation: web-16.0/web-16.0-web_m2x_options
Translate-URL: https://translation.odoo-community.org/projects/web-16-0/web-16-0-web_m2x_options/fr/
@vincent-hatakeyama @dyngnijs
[FIX] web_m2x_options: fix usage errors
2defbe7
@dyngnijs
Translated using Weblate (Portuguese (Brazil))
09f4fa0 
Currently translated at 100.0% (15 of 15 strings)

Translation: web-16.0/web-16.0-web_m2x_options
Translate-URL: https://translation.odoo-community.org/projects/web-16-0/web-16-0-web_m2x_options/pt_BR/
@pedrobaeza
Copy link
Member

I'm afraid not. You are removing the .deb in the last commit, while adding it in the previous one, doing 60 MB of diff. You should totally remove it in the same commit.

HaraldPanten
HaraldPanten suggested changes Jul 23, 2024
View reviewed changes
Copy link

@HaraldPanten HaraldPanten left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This is our proposal for the migration of the web_m2x_options module --> #2888

From our point of view, we think that the module needed a minor refactoring to improve it and remove options that have become obsolete in newest versions. If all the contributors agree, we propose to supersede this PR in favor of ours.

I think that this could help to move forward with the migration of this module.

What do you think @dyngnijs @CRogos @pedrobaeza ?

@pedrobaeza
Copy link
Member

@dyngnijs are you doing changes here by your own? What do you think about @HaraldPanten's proposal?

@dyngnijs
Copy link
Author

@pedrobaeza @HaraldPanten
I'm working on it alone. If the new PR works with the different widgets then its okay for me to supersede it. For example sol_product_many2one from the standard sale module.

@HaraldPanten
Copy link

@pedrobaeza @HaraldPanten I'm working on it alone. If the new PR works with the different widgets then its okay for me to supersede it. For example sol_product_many2one from the standard sale module.

Could you check ours, and see if it works well enough?

@HaraldPanten HaraldPanten mentioned this pull request Jul 23, 2024
Merged
@HaraldPanten
Copy link

@pedrobaeza considering the last comments, could we close this PR and keep working on this one? --> #2888

@pedrobaeza pedrobaeza closed this Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@remihb remihb remihb left review comments

@CRogos CRogos CRogos requested changes

@manuelregidor manuelregidor manuelregidor requested changes

@HaraldPanten HaraldPanten HaraldPanten requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
17.0
Development

Successfully merging this pull request may close these issues.