Hi,

First of all thanks for such lovely module.
about the POS Return.. I think we should look for order number then the sku of individual products. as sometimes, we give 10% discount but if we forget to notice it from the money receipt, we might cash back the full amount....

as computer should do things automatically and without error.... I think this could be useful.

Steps:
1> click on Return menu of POS
2> enter the order number then load the items was in that order... OR, let user put a sku on their own
3> load the price should be returned..
4> complete the refund process

we can mange manually, but I think this should be the way... at least to avoid typo :)

PR: https://github.com/AcroMedia/commerce_pos/pull/43

CommentFileSizeAuthor
#40 pos-return-order-serach-2772927-40.patch321.91 KBHubbs
#39 pos-return-order-serach-2772927-39.patch318.3 KBHubbs
#39 pos-return-order-serach-2772927-39-screenshot.png151.52 KBHubbs
#38 pos-return-order-search-2772927-38.patch11.4 KBHubbs
#37 Screen Shot 2017-06-20 at 6.14.59 PM.png134.1 KBshabana.navas
#37 pos-return-order-search-2772927-37.patch11.55 KBshabana.navas
#36 (Acro) Returns Modal Window - 1.0 - ap.jpg168.03 KBAnders Paulsen
#31 pos-return-order-search-2772927-31.patch316.44 KBshabana.navas
#30 pos-return-order-search-2772927-30.patch316.47 KBshabana.navas
#29 radios.png28.52 KBshabana.navas
#29 pos-return-order-search-2772927-29.patch316.47 KBshabana.navas
#28 (POS)-Return-UX-1.0-jp.jpg150.5 KBjjpoole
#25 pos-return-order-search-2772927-25.patch270.03 KBshabana.navas
#20 pos-return-order-search-2772927-19.patch269.92 KBshabana.navas
#16 pos-return-order-search-2772927-16.patch268.61 KBshabana.navas
#14 commerce_pos_return_validation_changes.patch1.68 KBAnonymous (not verified)
#14 Screen Shot 2017-04-26 at 9.36.00 PM.png226.76 KBAnonymous (not verified)
#12 pos-return-order-search-2772927-12.patch268.38 KBshabana.navas
#10 pos-return-order-search-2772927-10.patch270.67 KBsmccabe
#8 pos-return-order-search-2772927-8.patch262.67 KBsmccabe
#7 pos-return-order-search-2772927-7.patch234.14 KBshabana.navas
#6 pos-return-order-search-2772927-6.patch234.18 KBshabana.navas
#4 pos-return-order-search-2772927-4.patch520.35 KBshabana.navas
#3 search_by_order_no.png85.98 KBshabana.navas
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sharif.tanveer created an issue. See original summary.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
Status: Active » Needs work

I have taken an initial stab at the feature here: https://github.com/shabananavas/commerce_pos/tree/wip/pos-return-order-s.... Still left to do the pricing adjustment. The whole idea is that the item should be added with the actual price that the customer paid and not the default product price. But, unfortunately, I didn't get time to look into that yet and also need more input on how best to implement that.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
FileSize
search_by_order_no.png85.98 KB

A screenshot of how it would look.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
pos-return-order-search-2772927-4.patch520.35 KB

Patch attached.

smccabe’s picture

smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commented
Status: Needs review » Needs work

Hey shabana,

The patch above is buggered, it looks like it got some merge conflict files in there as well? i see some .orig files. Can you reroll a clean version for review plz.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
Status: Needs work » Needs review
FileSize
pos-return-order-search-2772927-6.patch234.18 KB

Updated patch.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
FileSize
pos-return-order-search-2772927-7.patch234.14 KB

Updated and clean patch against 7.x-2.x.

smccabe’s picture

smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commented
FileSize
pos-return-order-search-2772927-8.patch262.67 KB

Rerolled patch against -dev

smccabe’s picture

smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commented
Version: 7.x-2.0-beta5 » 7.x-2.x-dev
smccabe’s picture

smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commented
FileSize
pos-return-order-search-2772927-10.patch270.67 KB

Fixed up missing js file

smccabe’s picture

smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commented
Status: Needs review » Needs work

hey @shabana.navas with the new modal stuff from travis being in head now, can you reroll this. I took a look at it but there are a lot of conflicts and probably easier for you to sort out than me.

Thanks!

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
FileSize
pos-return-order-search-2772927-12.patch268.38 KB

Looks like part of the patch went in already in another commit. So, re-rolling the patch against the latest version with the pieces that were left out.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
Status: Needs work » Needs review
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
FileSize
Screen Shot 2017-04-26 at 9.36.00 PM.png226.76 KB
commerce_pos_return_validation_changes.patch1.68 KB

I've reviewed the patch from #12 and it appears to work. I was a little confused at the validation function though, and ended up replacing it with a built-in element validate callback. See the interdiff patch attached.

I also noticed some odd styling issues (see screenshot) but I wasn't sure of the best way to fix them.

Functionally, the patch is great and I think can be merged without further changes, but I had a few thoughts as I was reviewing it:

1) The order that is referenced isn't altered in any way as a result of the return. This might be confusing for people... Would it be better to alter the order that is being returned so that any items returned from it are removed from the order, and apply a negative payment transaction to the order reflecting the amount that was refunded? That way, we could reconcile orders that were returned and numbers should match up.

2) The Commerce POS Receipt module uses the order id on the receipt but labels it as 'Order Number' (which isn't actually accurate, it is using the id). This might be confusing because the help text for this new Order return feature says to enter the order id, not the number. We should probably change the Commerce POS Receipt module so that it properly labels the order id as an order id, not number.

3) I'm not sure if this is happening, but are we incrementing stock for returned items? Should we be?

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented

Thanks @wildkatana for cleaning up the validation. I have attached a new patch with those changes incorporated. As for your suggestions, I think @smccabe or @tbradbury are best to answer that.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
FileSize
pos-return-order-search-2772927-16.patch268.61 KB

Woops forgot to attach patch.

travis-bradbury’s picture

travis-bradbury CreditAttribution: travis-bradbury at Acro Commerce commented

The Commerce POS Receipt module uses the order id on the receipt but labels it as 'Order Number' (which isn't actually accurate, it is using the id). This might be confusing because the help text for this new Order return feature says to enter the order id, not the number.

I think order number is the way to go. Isn't a commerce_order's order number always set to the order_id anyway when it's not modified by something? Can we use order number instead of having the note about using ID instead of number? I think it's asking a lot to have a cashier understand what the heck an order ID is versus an order number.

I attempted to kick off a review on the PR but I'm not sure it's the current state of the patch because it might not have been re-rolled against the latest dev.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

I agree that Order Number is ideal for customer facing receipts and for cashiers as well. We should be consistent and use them in forms and elsewhere.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented

Thanks guy, I'll make the changes you mentioned. I haven't updated the PR, will do that as soon as I make the recommended changes.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
FileSize
pos-return-order-search-2772927-19.patch269.92 KB

New patch with changes recommended in #17 and #18 so that we're now searching by order number instead of ID.

I tried to update the PR but it had way too many conflicts and I kind of gave up :( and closed it out. Please use the patch to test this. Thanks.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
Issue summary: View changes
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

I didn't test it but I looked at the patch and can see that you made the changes I suggested. It would be nice to figure out the PR so we can review it more in-depth though... Have you tried making a new branch and applying your patch to it and making a new PR from that?

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
Issue summary: View changes

Created new PR here: https://github.com/AcroMedia/commerce_pos/pull/43

smccabe’s picture

smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commented
Issue tags: +rc1 blocker
shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
FileSize
pos-return-order-search-2772927-25.patch270.03 KB

More comment and code cleanup.

smccabe’s picture

smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commented

Code looks good, I'm gonna get jjpoole to go over it for bit of a UX review, for how to handle the 2 fields and the return button

smccabe’s picture

smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commented
Status: Needs review » Needs work

jjpoole is doing a UX mock for this, he'll post when he's done.

jjpoole’s picture

jjpoole CreditAttribution: jjpoole commented
FileSize
(POS)-Return-UX-1.0-jp.jpg150.5 KB

Here's a mockup. Toggle will focus the UX to one technique or the other, removing confusion around the two different ways to search.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
Status: Needs work » Needs review
FileSize
pos-return-order-search-2772927-29.patch316.47 KB
radios.png28.52 KB

Thanks @jpoole for the mockup. Updated patch is attached.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
FileSize
pos-return-order-search-2772927-30.patch316.47 KB

Fixed typo.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
FileSize
pos-return-order-search-2772927-31.patch316.44 KB

Oops wrong patch.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
jjpoole’s picture

jjpoole CreditAttribution: jjpoole commented

We should either remove or update the "Start your return..." text, as it doesn't mention the ability to search for order ID.

smccabe’s picture

smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commented
Status: Needs review » Needs work

Flipping back to needs work for the text change jjpoole mentioned.

Jason, did you also want to do any mockup for the modal that pops up for selecting the items from an order to return?

Anders Paulsen’s picture

Anders Paulsen
Location Victoria, BC
CreditAttribution: Anders Paulsen commented
FileSize
(Acro) Returns Modal Window - 1.0 - ap.jpg168.03 KB

Hey Shawn, I've added a mockup for that modal pop-up to the files below.

shabana.navas’s picture

shabana.navas CreditAttribution: shabana.navas at Acro Commerce commented
Status: Needs work » Needs review
FileSize
pos-return-order-search-2772927-37.patch11.55 KB
Screen Shot 2017-06-20 at 6.14.59 PM.png134.1 KB

Thanks Anders. I have styled as best as I could (see attached image). Latest patch again 2.x-dev is attached.

Hubbs’s picture

Hubbs CreditAttribution: Hubbs at Acro Commerce commented
FileSize
pos-return-order-search-2772927-38.patch11.4 KB

Updated patch. Not much changed in this as all I did was remove the addition of a mac .DS_STORE file.

I'm going to clean up a bit of CSS next. stay tuned.

Hubbs’s picture

Hubbs CreditAttribution: Hubbs at Acro Commerce commented
FileSize
pos-return-order-serach-2772927-39-screenshot.png151.52 KB
pos-return-order-serach-2772927-39.patch318.3 KB

Updated styles in this patch to closer match the design mockups by @Anders Paulsen and @jjpoole. I ended up adding a very small amount of padding on the image top and bottom. The design has the images without this spacing and no top/bottom border, so, it looked odd if the content within the box was taller than what the design showed, like if the title wrapped. You would end up with space top and bottom of the image but no border. To further this, adding a top/bottom border doubled up the top and bottom border thickness when the image was right against it the item boundaries, which also looked odd. Adding this extra padding made this a non-issue and I don't think detracts from the overall design.

Here's a screenshot:

There are a few things I'd still like to do, but, I ran out of time today.

  1. Some responsive fixes for the search fields at mobile.
  2. Add in the modal close and checkbox checkmark images based off of the designs.
Hubbs’s picture

Hubbs CreditAttribution: Hubbs at Acro Commerce commented
FileSize
pos-return-order-serach-2772927-40.patch321.91 KB

Updated patch that includes the remaining items I mentioned in #39.

Note: the modal close button was previously the X letter. I changed this in commerce_pos.common.inc to t('Close') for screen readers. The css hides the text and replaces it with an X image.

  • smccabe committed 2400fe3 on 7.x-2.x authored by Hubbs 
    Issue #2772927 by shabana.navas, Hubbs, smccabe, wildkatana, jjpoole,...
smccabe’s picture

smccabe CreditAttribution: smccabe as a volunteer and at Acro Commerce commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.