Skip to content

[18.0] [FIX] Aligns _find_record_check_access with core - #443

Merged
OCA-git-bot merged 1 commit into
OCA:18.0from
labiso-gmbh:18.0-FIX-web_editor_media_dialog_dms
Sep 5, 2025
Merged

[18.0] [FIX] Aligns _find_record_check_access with core#443
OCA-git-bot merged 1 commit into
OCA:18.0from
labiso-gmbh:18.0-FIX-web_editor_media_dialog_dms

Conversation

@llabusch93

@llabusch93 llabusch93 commented Sep 4, 2025

Copy link
Copy Markdown
Contributor

Updates the signature of the _find_record_check_access method to match recent changes in Odoo core. This ensures compatibility and prevents potential issues by correctly handling the new field parameter.

@llabusch93

Copy link
Copy Markdown
Contributor Author

Hello @pedrobaeza ,

This is the PR fixing my opened issue: #442

Cheers

@llabusch93
llabusch93 force-pushed the 18.0-FIX-web_editor_media_dialog_dms branch from fc06090 to c6fef4d Compare September 4, 2025 04:05
@pedrobaeza pedrobaeza added this to the 18.0 milestone Sep 4, 2025
Comment thread web_editor_media_dialog_dms/__manifest__.py Outdated
- [Tecnativa](https://www.tecnativa.com):
- Pedro M. Baeza
- Carlos Roca
- [LABISO GmbH](https://www.labiso.de)

@pedrobaeza pedrobaeza Sep 4, 2025

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.

As the contribution is so minimal, please don't include yourself here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As the contribution is so minimal, please done include yourself here.

I must express my serious concern regarding the handling of this matter. Initially, my issue was closed with what I perceived as a lack of consideration. Subsequently, our team invested considerable time and resources to resolve the problem, time that we could have dedicated to high-value client projects. Our commitment to the open-source community motivated this effort. Sure, the code change was very quick. But we identified it and reported it as an issue, which you closed. Then still taking the effort to fork this repository, fix it, and do a PR is time invested and also commitment. This must be valued.

Given this investment, the request to be omitted from the list of contributors is deeply perplexing. Contributing our expertise with the expectation of due credit is a fundamental tenet of our engagement with open source. Therefore, we cannot, in good conscience, consent to having our attribution removed.

Please understand that if this request is pressed, we will feel obligated to escalate the issue and seek a formal discussion with the OCA board regarding contribution and recognition policies. We believe a matter of such principle warrants a broader community dialogue.

@OSevangelist, I would be grateful for your thoughts on this situation.

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.

Sorry, but you didn't indicate in the issue that the extra module was installed. You just mentioned dms, and I therefore indicated all the traces that there was no problem on that module with the corresponding proofs.

If you think you have invested a lot of time, think in us creating and migrating the whole module, so that's why I say in comparison it's minimal. You will have the credit in the commit with your authorship, that will be preserved over time.

@llabusch93
llabusch93 force-pushed the 18.0-FIX-web_editor_media_dialog_dms branch from c6fef4d to acf4904 Compare September 4, 2025 06:35
@etobella

etobella commented Sep 4, 2025

Copy link
Copy Markdown
Member

I think there’s been some misunderstanding on both sides and I would like to bring some peace on this situation.

  • @llabusch93: thank you for taking the time to investigate, report, and provide a fix. Even when the code change itself is small, the effort to identify, validate, and contribute back is valuable to the community. That commitment is indeed recognized.
  • @pedrobaeza: I understand your perspective about relative effort when maintaining an entire module, and I agree that authorship credit in the commit history remains preserved.

To move forward, let’s keep the focus on collaboration and recognition:

  • The contribution will remain attributed in the commit log, as per OCA’s guidelines.
  • The contributor list is usually used for major changes in the modules.

Personally, the contribution file brings this kind of problems because it is not well defined when someone must be there or not. IMO, we (as a community) need to find a more objective way to define this file and avoid this kind of clashes.

@pedrobaeza

Copy link
Copy Markdown
Member

Yeah, I agree it's not clearly set what are the "merits" for being in the CONTRIBUTORS file, and anyway, maintainers and PSCs are the ones judging at the end. Anyway, as already said, the contributions are not hidden, being very strict preserving commit history on migrations and attributing properly in Git. On the same mood, the copyright lines at the beginning of each file has similar issues.

@llabusch93
llabusch93 force-pushed the 18.0-FIX-web_editor_media_dialog_dms branch from acf4904 to bbad9c9 Compare September 5, 2025 10:04
Aligns the overridden _find_record_check_access method with its updated signature in the Odoo core. This change ensures continued compatibility and proper access to DMS media files via access tokens.

Key Changes:
- Updates the method override to accept and pass the new 'field' parameter.
@llabusch93
llabusch93 force-pushed the 18.0-FIX-web_editor_media_dialog_dms branch from bbad9c9 to 28d5f81 Compare September 5, 2025 10:05
@llabusch93

Copy link
Copy Markdown
Contributor Author

Thank you for the thoughtful feedback. I completely agree that establishing clear guidelines in the contributors file would be a wonderful way to prevent these kinds of discussions from being necessary in the first place.

You've made a valid point about the commit history being preserved, which I had overlooked. Following your guidance, I have removed the changes to the contributors file and have also updated the commit message and pull request description. With these adjustments, the request now contains solely the code for the fix itself.

I hope this addresses all the concerns and that everything is now in order to be merged.

Warmly, and thanks again.

@etobella

etobella commented Sep 5, 2025

Copy link
Copy Markdown
Member

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 18.0-ocabot-merge-pr-443-by-etobella-bump-patch, awaiting test results.

@OCA-git-bot
OCA-git-bot merged commit 5d4cf14 into OCA:18.0 Sep 5, 2025
7 checks passed
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at e8fa11f. Thanks a lot for contributing to OCA. ❤️

@llabusch93
llabusch93 deleted the 18.0-FIX-web_editor_media_dialog_dms branch September 12, 2025 04:04
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.

5 participants