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

[MIG] document_url: Migration to 15.0 #353

Merged
merged 30 commits into from
Jul 25, 2022
Merged

Conversation

matiasperalta1
Copy link
Contributor

No description provided.

pedrobaeza and others added 26 commits March 30, 2022 14:59
[MIG] document_url

* bump version to 9.0.1.0.0 and make it installable
* Update JS structure
* Add css in order to be compliant with odoo style
* Add a README.rst and OCA conventions
* Improve selector
* add fr.po for fr translation
* open link with target='_blank'
Without this patch, when you hover on the menu item, its background color does not change, and its cursor is a text one.

With this patch, the menu item styling matches those available in all surrounding menus.
Replace char © by word Copyright
Change format in usage description
Updated by Update PO files to match POT (msgmerge) hook in Weblate.
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: knowledge-12.0/knowledge-12.0-document_url
Translate-URL: https://translation.odoo-community.org/projects/knowledge-12-0/knowledge-12-0-document_url/
Currently translated at 100.0% (14 of 14 strings)

Translation: knowledge-12.0/knowledge-12.0-document_url
Translate-URL: https://translation.odoo-community.org/projects/knowledge-12-0/knowledge-12-0-document_url/hr/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: knowledge-13.0/knowledge-13.0-document_url
Translate-URL: https://translation.odoo-community.org/projects/knowledge-13-0/knowledge-13-0-document_url/
@matiasperalta1 matiasperalta1 force-pushed the 15.0-t-25911-mnp branch 3 times, most recently from a6c6813 to 9044c8f Compare June 16, 2022 18:03
@nicomacr nicomacr mentioned this pull request Jun 28, 2022
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.

See this image for my comments:

Selección_007

You can compare with the module in v14:

Selección_009

@matiasperalta1
Copy link
Contributor Author

@victoralmau Hi Victor. I contact you to ask you a question for this module. I'm migrating this module to v15 and I have seen that some changes were made by you in the migration to v14. I am trying to use the mimetype "application/link" but it is not working for me because I think it is invalid. How have you made it work?
Thank you very much and I hope your answer.

@victoralmau
Copy link
Member

Hi, the approach is correct. I think the first thing to do is to add all the changes from v14 to the commit history. These changes are explained in the v14 migration PR: #306

DavidBForgeFlow and others added 3 commits July 18, 2022 16:19
[MIG] document_url: Migration to 14.0
Define the add attachment and add URL buttons on the same line.
Allow downloading of url type attachments by clicking on the icon or the name.
Define the name of the url type attachments as links (allows to open it in a new tab).
Hide the download button for url attachments.
Force to set mimetype to "application/link" for url attachments.
Displays a link icon for url attachments.
TT30263
@pedrobaeza
Copy link
Member

/ocabot migration document_url

@OCA-git-bot OCA-git-bot added this to the 15.0 milestone Jul 20, 2022
@OCA-git-bot
Copy link
Contributor

The migration issue (#308) has been updated to reference the current pull request.
however, a previous pull request was referenced : #337.
Perhaps you should check that there is no duplicate work.
CC : @JoelZilli

@pedrobaeza
Copy link
Member

Everything is working now except having "Add attachments" and "Add URL" in 2 lines instead of one. Can you please check it?

@pedrobaeza
Copy link
Member

The migration issue (#308) has been updated to reference the current pull request. however, a previous pull request was referenced : #337. Perhaps you should check that there is no duplicate work. CC : @JoelZilli

@legalsylvain it seems the new check is not correct, as the referenced PR is closed

Copy link
Member

@victoralmau victoralmau left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the work.

  • Do not delete url.js file, make the necessary changes to reduce diff in commit
  • Squash administrative commits

Except for the comment in #353 (comment), everything would be correct.

@pedrobaeza
Copy link
Member

Pre-commit is failing.

@matiasperalta1
Copy link
Contributor Author

@pedrobaeza Pre-commit Fixed

@pedrobaeza
Copy link
Member

Thanks for the contribution

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 15.0-ocabot-merge-pr-353-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit cc020fb into OCA:15.0 Jul 25, 2022
@OCA-git-bot
Copy link
Contributor

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

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.