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.

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

[14.0][FIX] dms: Remove _get_share_url() function because is not necessary to override nothing #79

Closed
wants to merge 1 commit into from

Conversation

victoralmau
Copy link
Member

@victoralmau victoralmau commented May 24, 2021

Remove _get_share_url() function because is not necessary to override nothing (default work fine according to share wizard and auto-add access_token in link).

It's need to apply in 13.0 too.

Please @pedrobaeza and @chienandalu can you review it?

@Tecnativa TT29820

@pedrobaeza pedrobaeza added this to the 14.0 milestone May 24, 2021
@pedrobaeza
Copy link
Member

One question: shouldn't be the token then populated only if the new argument is true?

@victoralmau
Copy link
Member Author

One question: shouldn't be the token then populated only if the new argument is true?

IMO is not necessary because actually code use always access_token (similar to odoo/odoo@a55f125#diff-b90ce1ef695264f5890e01458a99eb2b7ab9679df4cfdbec5a0d4c3f156f9e67R54)

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

That's not what I'm reading in the link you have passed me, so please follow the same steps as in sale:

  • Gather params in the variables and then call to encode_params.
  • Don't populate the access token if share_token is False.

…to override nothing (default work fine according to share wizard and auto-add access_token in link).
@victoralmau victoralmau changed the title [14.0][FIX] dms: Improve _get_share_url() function to add share_token parameter [14.0][FIX] dms: Remove _get_share_url() function because is not necessary to override nothing May 26, 2021
@victoralmau
Copy link
Member Author

According to analysis related to _get_share_url() in OCA/contract#680 and test share button, share link and button in mail, my conclusion is that _get_share_url() is not necessary (default functionality is correct in dms use case).

@victoralmau
Copy link
Member Author

It's not necessary now because these changes have apllied in #93 (comment)

@pedrobaeza pedrobaeza deleted the 14.0-fix-dms-share_url branch August 8, 2023 08:41
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