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.

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

[17.0] [ADD] pos_order_attachment #1137

Open
wants to merge 2 commits into
base: 17.0
Choose a base branch
from
Open

[17.0] [ADD] pos_order_attachment #1137

wants to merge 2 commits into from

Conversation

etobella
Copy link
Member

@etobella etobella commented Jan 22, 2024 •
edited
Loading

This addon allows to manage Attachments from Orders on POS Frontend

Known bugs:

  • preview does not work properly
  • Improve view of non images attachments
  • Button quantity is not refreshed properly
  • We shouldn't store attachments of not validated Orders.

@cvinh

@etobella etobella force-pushed the 17.0-add-pos_attachment branch from 4f2e30c to be034fc Compare January 22, 2024 20:30
legalsylvain
legalsylvain reviewed Jan 22, 2024
View reviewed changes
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.

Partial test. Some remarks inline.
also I added an attahcment in the Pos, and the quantity of attachment is not updated.

(note : I didn't saw that the PR was in draft, so sorry for the noice, if it's WIP).

pos_attachment/static/src/app/screens/product_screen/attachments_button.esm.js Outdated
Comment on lines 24 to 37
async click() {
const order = this.pos.get_order();
order.save_to_db();
this.env.services.ui.block();
const syncOrderResult = await this.pos.push_single_order(order);
this.env.services.ui.unblock();
if (syncOrderResult instanceof ConnectionLostError) {
return;
} else if (!syncOrderResult) {
return;
}
await this.popup.add(AttachmentPopup, {order: order});
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you save / push your order before displaying the popup.

weird step to reproduce :

  • create an order product 100€.
  • go to payment screen set a payment of 100€.
  • go back to the previous scren and click on attachments.

Go to the backoffice :
-> the order is marked as paid. (even if the user didn't click on "validate payment".)

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I want to avoid to store the attachments inside the Database. It is certainly an issue, maybe it has more sense to add this at already validated orders. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

well, I think attachment should be saved, when orders are paid (/invoiced), indeed (or saved in draft) Otherwise, you'll have side effect, like this one I think.

@etobella etobella force-pushed the 17.0-add-pos_attachment branch from be034fc to baf8ab3 Compare January 22, 2024 20:43
@cvinh
Copy link
Contributor

cvinh commented Jan 23, 2024

I think it's better to rename to pos_order_attachment (in case we have other modules in pos with attachments... ie pos_customer_attachment...)

@etobella etobella changed the title [ADD] pos_attachment [ADD] pos_order_attachment Jan 24, 2024
@etobella etobella marked this pull request as ready for review January 24, 2024 09:18
@etobella
Copy link
Member Author

Renamed to pos_order_attachment @cvinh

Attachments can be added only on validated orders

@etobella etobella force-pushed the 17.0-add-pos_attachment branch 2 times, most recently from 4813eae to 21ed459 Compare January 29, 2024 10:35
@etobella etobella force-pushed the 17.0-add-pos_attachment branch from 21ed459 to 8e1876a Compare January 29, 2024 11:02
@etobella etobella force-pushed the 17.0-add-pos_attachment branch from 7990ab8 to 3cca2e4 Compare February 20, 2024 21:14
@legalsylvain legalsylvain changed the title [ADD] pos_order_attachment [16.0] [ADD] pos_order_attachment Feb 21, 2024
@legalsylvain legalsylvain added this to the 16.0 milestone Feb 21, 2024
@legalsylvain legalsylvain changed the title [16.0] [ADD] pos_order_attachment [17.0] [ADD] pos_order_attachment Feb 21, 2024
@legalsylvain legalsylvain modified the milestones: 16.0, 17.0 Feb 21, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 8, 2024

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Sep 8, 2024
@github-actions github-actions bot closed this Oct 13, 2024
@vehi-invitu
Copy link

@etobella can you reopen, please.

@etobella etobella reopened this Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@legalsylvain legalsylvain legalsylvain left review comments

@cvinh cvinh cvinh approved these changes

Assignees
No one assigned
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Milestone
17.0
Development

Successfully merging this pull request may close these issues.
4 participants
@etobella @cvinh @vehi-invitu @legalsylvain