Skip to content

Conversation

@bso-odoo
Copy link

@bso-odoo bso-odoo commented Feb 4, 2025

No description provided.

@robodoo
Copy link

robodoo commented Feb 4, 2025

This PR targets the un-managed branch odoo-dev/odoo:master-mysterious-egg, it needs to be retargeted before it can be merged.

@bso-odoo bso-odoo force-pushed the master-mysterious-egg-bso-2 branch 4 times, most recently from 4b2e9bf to a6b4c92 Compare February 4, 2025 14:17
Copy link

Choose a reason for hiding this comment

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

be careful, the dynamicsnippetoption component is destroyed and recreated possibly many times (for ex, when switching from custom tab to block tab and back), so we don't want to refetch dynamic snippets everytime. i guess a better solution would be to move that in a plugin

Copy link
Author

Choose a reason for hiding this comment

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

@ged-odoo As far as I undestood so far from the answers I received on this topic, there is no async life-cycle entry point in plugins, and their shared functions are not supposed to be async either... and if I keep the info in a Promise in the plugin, I doubt the template rendering allows await.

Copy link

Choose a reason for hiding this comment

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

in a plugin, you don't have to have a async lifecycle. you can just work with promises:

class DynamicSnippetOptionPlugin extends Plugin {
    static id = "DynamicSnippetOption";
    resources = {
        builder_options: [
            withSequence(10, {
                OptionComponent: DynamicSnippetOption,
                props: { loadFilters: () => this.fetchDynamicFilters() },
                selector: ".s_dynamic_snippet",
            }),
        ],
        builder_actions: this.getActions(),
    };

  setup() {
    this.filters = null;
  }
  fetchDynamicFilters() {
    if (!this.filters) {
      this.filters = this._fetchDynamicFilters();
    }
    return this.filters;
  }
  async _fetchDynamicFilters() {
    const filters = await rpc(...);
     ....
  }
}

and in your component:

class DynamicSnippetOption extends Component {
  onWillStart() {
    this.filters = await this.props.loadFilters();
  }

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I did something similar, but with a cache based on the parameters since the plugin is a singleton. And I also made those shared methods so that the other kinds of dynamic snippet options can also use them.

@bso-odoo bso-odoo force-pushed the master-mysterious-egg-bso-2 branch 5 times, most recently from fa174dd to 3e6b990 Compare February 5, 2025 12:45
@bso-odoo bso-odoo marked this pull request as ready for review February 5, 2025 12:45
@robodoo
Copy link

robodoo commented Feb 5, 2025

This PR targets the un-managed branch odoo-dev/odoo:master-mysterious-egg, it needs to be retargeted before it can be merged.

@bso-odoo bso-odoo force-pushed the master-mysterious-egg-bso-2 branch from 3e6b990 to cb54b20 Compare February 5, 2025 12:53
Comment on lines 372 to 383

Choose a reason for hiding this comment

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

if (unit) {
if(saveUnit) {
actionValue = convertNumericToUnit(...) + saveUnit
} else {
actionValue = actionValue + unit
}

Copy link
Author

Choose a reason for hiding this comment

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

Also added test

@bso-odoo bso-odoo force-pushed the master-mysterious-egg-bso-2 branch 2 times, most recently from 53107df to 0f4332e Compare February 6, 2025 10:27

Choose a reason for hiding this comment

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

you don t use template_opt ?

Copy link
Author

Choose a reason for hiding this comment

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

Used by future xpaths

Choose a reason for hiding this comment

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

you don t use filter_opt ?

Copy link
Author

Choose a reason for hiding this comment

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

Used by future xpaths

Comment on lines +16 to +17

Choose a reason for hiding this comment

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

<BuilderSelectItem t-if=""template.thumb" is supported if you want to remove 2 lines

Copy link
Author

Choose a reason for hiding this comment

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

I prefer to be able to xpath "inside t-if" in case the image is not just what we want

@bso-odoo bso-odoo force-pushed the master-mysterious-egg-bso-2 branch from 0f4332e to 93d4abc Compare February 6, 2025 13:56
@FrancoisGe FrancoisGe merged commit f8c7c7e into master-mysterious-egg Feb 6, 2025
@FrancoisGe FrancoisGe deleted the master-mysterious-egg-bso-2 branch February 6, 2025 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants