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

12.0 IMP pos_order_mgmt #378

Closed
wants to merge 3 commits into from
Closed

12.0 IMP pos_order_mgmt #378

wants to merge 3 commits into from

Conversation

ivantodorovich
Copy link
Contributor

@PierrickBrun @legalsylvain Hi guys! Thanks for the effort on the other PR, btw.

here are two small improvements on action_print:

  • Print invoices instead of tickets, if the order was invoiced
  • Destroy the order after printing, so that it's removed from the browser's localStorage

…n order, if the user refreshes the browser the order will load again. And this will happen for every previously printed order in the session.
Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Hi @ivantodorovich. Thanks of a lot for your contribution.

  • Destroy the order after printing, so that it's removed from the browser's localStorage

💯 ! You fix something I didn't understood, when I ported this module. Thanks ;-)

  • Print invoices instead of tickets, if the order was invoiced

I'm in favour in a more modular approach. Even it is an invoiced order, maybe the cashier want to print the bill with his thermical printer. (mainly because perhaps, the cashier only has a thermical printer) So, I propose to make the following changes :

  • Add a new button print_invoice (displayed only for invoiced order).

  • add a new function action_print_invoice with the code you wrote.

      action_print_invoice: function (order_data, order) {
          return this.pos.chrome.do_action('point_of_sale.pos_invoice_report', {
              additional_context: { active_ids: [order_data.id] }
          }
      }
    

What do you think ?

kind regards.

@ivantodorovich
Copy link
Contributor Author

@legalsylvain Actually that was my first approach, but adding another button made the order list view ugly and it just seemed unnecessary.

How about now? It's similar to the odoo standard: if you are Invoicing the order you get both the receipt and the invoice.

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Hi !
Now is perfect. printing both is the best behaviour IMO, so cashier can give the bill, or print the downloaded pdf.
kind regards.
LGTM, code review, no test.

/ocabot merge patch

@legalsylvain
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Rebased to 12.0-ocabot-merge-pr-378-by-legalsylvain-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jul 24, 2019
Signed-off-by legalsylvain
@OCA-git-bot
Copy link
Contributor

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

PS: Don't worry if GitHub says there are unmerged commits: it is due to a rebase before merge. All commits of this PR have been merged into 12.0.

@ivantodorovich ivantodorovich deleted the 12.0-imp-pos_mgmt branch July 24, 2019 14:29
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.

None yet

3 participants