Skip to content

[IMP] inventory: print shipping label #8658

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
Closed
wants to merge 1 commit into from
Closed

[IMP] inventory: print shipping label #8658

wants to merge 1 commit into from

Conversation

Felicious
Copy link
Contributor

@Felicious Felicious commented Apr 10, 2024 •
edited
Loading

Summary

  1. Overhaul 8 year old doc about generating shipping labels
  2. Merge after 3rd party shipping doc because I reused the images 👀

FWport: yes!

Task

@robodoo
Copy link
Collaborator

robodoo commented Apr 10, 2024

This PR targets the un-managed branch odoo/documentation:16.0-inventory-third-party-shipper-feku, it needs to be retargeted before it can be merged.

@Felicious Felicious force-pushed the 16.0-inventory-third-party-shipper-feku branch 2 times, most recently from aab59bb to ed27fa1 Compare April 16, 2024 20:31
@Felicious Felicious force-pushed the 16.0-inventory-shipping-labels-feku branch from 1aa94d2 to e18ee19 Compare April 17, 2024 00:01
@Felicious Felicious requested a review from a team April 17, 2024 00:02
@Felicious
Copy link
Contributor Author

Hello @odoo/inventory-doc-review , this PR is ready for review!! Thank you in advance for the help :)

@brse-odoo
Copy link
Contributor

@Felicious I'll review this right away!

@Felicious Felicious force-pushed the 16.0-inventory-third-party-shipper-feku branch from 6e6b9e9 to 7f253ff Compare April 18, 2024 02:12
brse-odoo
brse-odoo approved these changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@brse-odoo brse-odoo left a comment

Choose a reason for hiding this comment

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

@Felicious All done reviewing this, great job! Approving with very few suggestions, let me know if you have any questions, Thanks!

@Felicious Felicious force-pushed the 16.0-inventory-third-party-shipper-feku branch from 87dd60c to 163883d Compare April 18, 2024 21:47
@Felicious Felicious force-pushed the 16.0-inventory-shipping-labels-feku branch from 57d2dfb to d80565b Compare April 18, 2024 22:48
Felicious
Felicious commented Apr 18, 2024
View reviewed changes
...tions/inventory_and_mrp/inventory/shipping_receiving/advanced_operations_shipping/labels.rst Outdated
Comment on lines 14 to 20
To generate labels for a third-party shipping carrier, install the shipping connector, configure and
activate the delivery method, and provide the company's :ref:`source address
<inventory/shipping_receiving/configure-source-address>` and :ref:`product weights
<inventory/shipping_receiving/configure-weight>`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@odoo/us-doc-review, I wanted to get your opinion on how short to keep this Configuration section.

Technically, the configuration of this doc is the exact same as the third_party_shipper's (the PR that's supposed to be merged before this one). The only difference is that we set the "integration level" to "Get Rate and Create Shipment" in the delivery method section.

I chose to keep the shipping connector installation part in this doc because it made sense, but it can be taken out. Should I keep the "Install shipping connector" section in the doc, or should I just link to the other doc?

I decided to not include the source address and product weights configuration in the doc, as this doc and the third_party_shipper one starts to become really similar. Instead, in the "Add shipping on quotation" section, I mention that the source address and product weights must be configured for the shipping connector to work properly. Do you think mentioning it there is enough? Would love to know your thoughts!

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @Felicious, this is a really good conversation to have– I like that you are trying to reduce writing duplicate information by leveraging the modular nature of docs. After looking at both this PR and #8634, I think that you can remove a lot of the same/similar content you have in this PR (#8658), here is my reasoning:

In the Configuration section of #8634, you already detail (with an important admonition) the Integration Level needed for generating shipping labels. I think this is a perfect use of an admonition and internal reference.

So for the Configuration section of this PR (#8658), I think you can get by with just stating something like the following, removing the Install shipping connector, Delivery method, and Production environment sections from this PR:

Configuration
=============

To generate labels for a third-party shipping carrier, first :doc:`install the third party shipping connector
<../setup_configuration/third_party_shipper>`. Then configure and activate the :ref:`delivery method
<inventory-shipping-receiving-configure-delivery-method>`, being sure to set the :guilabel:`Integration
Level` to :guilabel:`Get Rate and Create Shipment` to generate shipping labels. Finally, provide the
company's :ref:`source address <inventory/shipping_receiving/configure-source-address>` and
:ref:`product weights <inventory/shipping_receiving/configure-weight>`. 

.. seealso::
   :doc:`../setup_configuration/third_party_shipper`

Lastly, I think your important admonition in the Add shipping on quotation works well, and mentioning it there is enough IMO.

@Felicious Felicious requested a review from a team April 18, 2024 22:59
@Felicious Felicious added the 5 label Apr 18, 2024
@Felicious Felicious self-assigned this Apr 18, 2024
@Felicious Felicious force-pushed the 16.0-inventory-shipping-labels-feku branch from d80565b to c9c178c Compare April 18, 2024 23:04
@Felicious Felicious force-pushed the 16.0-inventory-third-party-shipper-feku branch from 7afc52d to db9991d Compare April 19, 2024 17:19
ksc-odoo
ksc-odoo approved these changes Apr 22, 2024
View reviewed changes
Copy link
Contributor

@ksc-odoo ksc-odoo left a comment

Choose a reason for hiding this comment

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

Hey @Felicious -- Just finished my Final Review. Great job! Only have a handful of minor suggestions that require your attention. Once you implement the necessary changes, feel free to tag this for Tech Review. Thanks! 👍

@Felicious Felicious force-pushed the 16.0-inventory-shipping-labels-feku branch from 6f49308 to d10e09c Compare April 22, 2024 18:04
ksc-odoo
ksc-odoo approved these changes Apr 22, 2024
View reviewed changes
Copy link
Contributor

@ksc-odoo ksc-odoo left a comment

Choose a reason for hiding this comment

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

@Felicious -- looks good to me! Just a couple SUPER SMALL suggestions that require your attention 👍

@Felicious Felicious force-pushed the 16.0-inventory-shipping-labels-feku branch from 553074a to c020114 Compare April 22, 2024 20:10
@Felicious Felicious changed the base branch from 16.0-inventory-third-party-shipper-feku to 16.0 April 22, 2024 20:10
@Felicious Felicious requested a review from samueljlieber April 22, 2024 20:10
@robodoo
Copy link
Collaborator

robodoo commented Apr 22, 2024

Pull request status dashboard.

@Felicious
Copy link
Contributor Author

Hi @samueljlieber ! This PR has passed final review and is ready for your tech review, whenever you get the chance (:

@C3POdoo C3POdoo requested a review from a team April 22, 2024 20:11
@Felicious Felicious removed the request for review from a team April 22, 2024 20:17
@Felicious Felicious force-pushed the 16.0-inventory-shipping-labels-feku branch from c020114 to eb38285 Compare April 22, 2024 20:19
samueljlieber
samueljlieber approved these changes Apr 23, 2024
View reviewed changes
Copy link
Contributor

@samueljlieber samueljlieber left a comment

Choose a reason for hiding this comment

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

Hi @Felicious! Great job improving the content on this Inventory doc. Your changes in this PR look good to me– can you just double check the images I marked as they seem to be extra and unused, if so please remove before merging! Approving and delegating to you.
.....
@robodoo delegate=Felicious

...ventory/shipping_receiving/setup_configuration/third_party_shipper/fedex-shipping-config.png Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra unused image? 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching these extra, unused images! They were some stuff leftover from targetting the "overhaul 3rd party shipper" PR that got lost in the revisions. Removed these (:

Appreciate your attention to detail very much 😊

...ntory/shipping_receiving/setup_configuration/third_party_shipper/publish-delivery-method.png Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra unused image? 🙂

...y_and_mrp/inventory/shipping_receiving/setup_configuration/third_party_shipper/published.png Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra unused image? 🙂

@Felicious Felicious force-pushed the 16.0-inventory-shipping-labels-feku branch from eb38285 to 0a7e77b Compare April 23, 2024 17:31
@Felicious @brse-odoo @ksc-odoo
[IMP] inventory: print shipping label
f094c39 
Co-authored-by: brse-odoo <brse@odoo.com>
Co-authored-by: ksc-odoo <73958186+ksc-odoo@users.noreply.github.com>
@Felicious Felicious force-pushed the 16.0-inventory-shipping-labels-feku branch from 0a7e77b to f094c39 Compare April 23, 2024 17:33
@Felicious
Copy link
Contributor Author

@robodoo r+

robodoo pushed a commit that referenced this pull request Apr 23, 2024
@Felicious @brse-odoo @ksc-odoo
[IMP] inventory: print shipping label
ecc993e 
closes #8658

Signed-off-by: Felicia Kuan (feku) <feku@odoo.com>
Co-authored-by: brse-odoo <brse@odoo.com>
Co-authored-by: ksc-odoo <73958186+ksc-odoo@users.noreply.github.com>
@fw-bot fw-bot mentioned this pull request Apr 23, 2024
Closed
@robodoo robodoo closed this Apr 23, 2024
This was referenced Apr 23, 2024
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samueljlieber samueljlieber samueljlieber approved these changes

+2 more reviewers

@ksc-odoo ksc-odoo ksc-odoo approved these changes

@brse-odoo brse-odoo brse-odoo approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@Felicious Felicious

Labels
5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Felicious @robodoo @brse-odoo @samueljlieber @ksc-odoo