[Spree Upgrade] Fix admin new order page #3194

HugsDaniel · 2018-12-07T10:37:43Z

What? Why?

Closes #3109 and #3107 and #2416
The spec in #3107 should be green.

@HugsDaniel said:
New new/edit order page in back office! Upgraded Spree views to 2.0 and applied overrides.
There's no error anymore for blank line_items (it was added with deface), and line items can be added.
I used Spree default routes for these actions (see in _routes.html.erb to make this work.

@luisramos0 said:
Regarding the fix to #2416 by adapting app/views/spree/admin/variants/_autocomplete.js.erb and the respective search.rabl file: this is a quick fix to the problem, there's plenty we could improve here, I suggest we leave for after the upgrade, I created this tech debt issue #3440

Remaining work to be done in follow up PRs:

the display of order adjustments needs to be reviewed (adjustments included in price are not showing) [Spree Upgrade] Included vs not included Adjustments in the edit order page (1 broken test) #2940 (fixed in PR [Spree Upgrade] Fix adjustments sections in admin order edit page #3445)
fix the flow between order details page and customer details page [Spree Upgrade] Changing distributor and OC in the Edit Orders page (3 broken specs) #3103 (fixed in PR [Spree Upgrade] Fix admin new order page flow (from order details to customer details) #3436)
scope variants when adding them to the order api/shipments (and probably get rid of admin/line_items_controller) [Spree Upgrade] Scope variants when they are added in the edit order page through api/shipments (2 broken specs) #3422 (fixed in PR [Spree Upgrade] Adapt api shipments controller to OFN by scoping variants before each action #3441)

For the review, I recommend reviewers to follow the commits in order.

luisramos0

awesome, keep going!

Two JS errors coming from _form.html.haml:30 :

If you are stuck, please provide full details on the two JS errors you are seeing.

There are a lot of JS function that could/should be converted into Angular's

I wouldnt worry about converting everything to angular, at least not yet.

app/views/spree/admin/shared/_routes.html.erb

app/views/spree/admin/variants/_autocomplete.js.erb

luisramos0 · 2018-12-07T11:04:25Z

app/views/spree/admin/variants/_autocomplete.js.erb

+                  <button class="add_variant no-text icon-plus icon_link with-tip" data-stock-location-id="{{this.stock_location_id}}" title="Add" data-action="add"></button>
+                </td>
+              {{else}}
+                <td><%= Spree.t(:out_of_stock) %></td>


all Spree.t entries should be replaced by normal lazy loaded entries (may need to add these to our en.yml file).

mkllnk · 2018-12-07T11:57:35Z

Oh no, we are working on the same thing. Have a look at #2938 (comment). Different approach.

HugsDaniel · 2018-12-07T12:29:09Z

@mkllnk are you just working on the translation issue or on the whole new page ?

mkllnk · 2018-12-07T12:32:46Z

Just on the translation issue. I'm trying to get ./spec/features/admin/orders_spec.rb:48 passing.

mkllnk · 2018-12-07T12:34:41Z

But I am changing app/views/spree/admin/variants/_autocomplete.js.erb for that while you are replacing it. So there is a conflict in our work.

mkllnk · 2018-12-07T12:50:30Z

I opened my pull request: #3195. You can see what I did.

I think it is quite useful to have the count on hand in that view. But maybe not essential? Should we just x the on hand display? @luisramos0

@HugsDaniel Where did you get the original view from? I though Spree 2 switched to using the API and doesn't have that view any more. Did you import the Spree 1 view?

HugsDaniel · 2018-12-07T13:42:58Z

_autocomplete.js.erb you mean ? It's in Spree 2

mkllnk · 2018-12-10T23:52:52Z

Ah, sorry for the confusion @HugsDaniel. I mixed it up with app/views/spree/admin/variants/search.rabl which is gone in Spree 2.

luisramos0 · 2019-01-08T19:01:34Z

Hugs I see this PR is updating adjustments on the order page. There's an issue for two of the specs here: #2940
We need to make sure we know where 1. adjustments included in the line items and 2. shipping costs are being displayed.

app/views/spree/admin/orders/_form.html.haml

luisramos0 · 2019-01-31T15:46:43Z

@HugsDaniel I pushed a new commit that fixes line item controller spec

spec/features/admin/orders_spec.rb

luisramos0 · 2019-01-31T22:38:53Z

No wonder @HugsDaniel was a bit stuck with this one. What a difficult job!
I have updated the description above: all the things to be done after this PR are mapped in other issues now.
We should review and merge this PR as is.

Add missing form tag and OC and shops injectors on order form to make the OC field, the distributor field and the update button work

…pec in admin orders spec

mkllnk

I can't comprehend all the view changes. I would need to look into what's changed in Spree and look into our modifications. That would be almost as much work as doing it again. ;-)

It looks all good though. If there is something important missing, we should find it in the testing phase and write specs for it.

spec/features/admin/orders_spec.rb

sauloperez · 2019-02-07T13:38:09Z

spec/features/admin/orders_spec.rb

+    # Moves the order back to the cart state
+    #   A nil user keeps the order in the cart state
+    #   Even if the edit page tries to automatically progress the order workflow
+    @order.state = 'cart'; @order.user = nil; @order.completed_at = nil; @order.save


why not in multiple lines? we can even extract a helper method where we can document in detail why this is needed.

👍
I keep it as multilines in the same place without extracting to method, I think the test is simple enough to take this.
see new commit.

sauloperez · 2019-02-07T13:38:14Z

spec/features/admin/orders_spec.rb

@@ -101,18 +101,18 @@ def new_order_with_distribution(distributor, order_cycle)
  scenario "displays error when incorrect distribution for products is chosen" do
    d = create(:distributor_enterprise)
    oc = create(:simple_order_cycle, distributors: [d])
-    puts d.name
-    puts @distributor.name


spec/features/admin/orders_spec.rb

luisramos0 · 2019-02-12T09:56:58Z

hurray!!!!

HugsDaniel self-assigned this Dec 7, 2018

luisramos0 reviewed Dec 7, 2018

View reviewed changes

mkllnk mentioned this pull request Dec 7, 2018

[Spree Upgrade] 2938 Fix Create Order in Admin - missing translation #3195

Merged

HugsDaniel mentioned this pull request Dec 14, 2018

[Spree Upgrade] Replace search to add products to order as admin #2416

Closed

luisramos0 mentioned this pull request Jan 8, 2019

[Spree Upgrade] Included vs not included Adjustments in the edit order page (1 broken test) #2940

Closed

HugsDaniel force-pushed the 3109-fix-orders-new branch from af92634 to cd21223 Compare January 10, 2019 14:52

HugsDaniel force-pushed the 3109-fix-orders-new branch from cd21223 to ec78022 Compare January 24, 2019 09:04

HugsDaniel commented Jan 31, 2019

View reviewed changes

app/views/spree/admin/orders/_form.html.haml Show resolved Hide resolved

HugsDaniel force-pushed the 3109-fix-orders-new branch from ec78022 to ae32e82 Compare January 31, 2019 11:57

Upgrade views to Spree 2 and apply overrides

fa1525b

HugsDaniel force-pushed the 3109-fix-orders-new branch from ae32e82 to 88fab3d Compare January 31, 2019 11:59

luisramos0 reviewed Jan 31, 2019

View reviewed changes

spec/features/admin/orders_spec.rb Outdated Show resolved Hide resolved

luisramos0 reviewed Jan 31, 2019

View reviewed changes

spec/features/admin/orders_spec.rb Outdated Show resolved Hide resolved

HugsDaniel force-pushed the 3109-fix-orders-new branch 2 times, most recently from 9b38e9d to acc6fbd Compare January 31, 2019 16:10

luisramos0 force-pushed the 3109-fix-orders-new branch from acc6fbd to 9ec0e97 Compare January 31, 2019 17:01

This was referenced Jan 31, 2019

[Spree Upgrade] Changing distributor and OC in the Edit Orders page (3 broken specs) #3103

Closed

[Spree Upgrade] Admin - Fix new Order page (no broken tests, yet) #3109

Closed

luisramos0 force-pushed the 3109-fix-orders-new branch from 4c3df43 to 79224f6 Compare January 31, 2019 21:29

luisramos0 self-assigned this Jan 31, 2019

luisramos0 changed the title ~~[WIP] [Spree Upgrade] Fix admin new order page~~ [Spree Upgrade] Fix admin new order page Jan 31, 2019

luisramos0 force-pushed the 3109-fix-orders-new branch from 79224f6 to 701a3ad Compare January 31, 2019 22:36

luisramos0 force-pushed the 3109-fix-orders-new branch 2 times, most recently from e26801c to 4d784c2 Compare February 3, 2019 17:19

luisramos0 mentioned this pull request Feb 3, 2019

[Spree Upgrade] Fix admin new order page flow (from order details to customer details) #3436

Merged

luisramos0 changed the title ~~[Spree Upgrade] Fix admin new order page~~ [Spree Upgrade] WIP WIPFix admin new order page Feb 4, 2019

HugsDaniel and others added 6 commits February 4, 2019 18:56

Use Spree routes

ae799eb

Fix failing specs due to Spree 2's new order admin page

c823f94

Add missing form tag and OC and shops injectors on order form to make the OC field, the distributor field and the update button work

Fix admin orders spec non tax adjustments by adapting to new view

c612df5

Fix distributor change spec in admin orders spec

e5fdc89

Adapt customer search override to spree v2 and fix customer details s…

698ba46

…pec in admin orders spec

Adapt variants auto complete to spree v2 code with shipments

d598831

luisramos0 force-pushed the 3109-fix-orders-new branch from 4d784c2 to d598831 Compare February 4, 2019 19:00

luisramos0 changed the title ~~[Spree Upgrade] WIP WIPFix admin new order page~~ [Spree Upgrade] Fix admin new order page Feb 4, 2019

This was referenced Feb 5, 2019

Improve Variants autocomplete solution #3440

Open

[Spree Upgrade] Adapt api shipments controller to OFN by scoping variants before each action #3441

Merged

luisramos0 requested review from sauloperez and mkllnk February 6, 2019 11:36

mkllnk approved these changes Feb 7, 2019

View reviewed changes

spec/features/admin/orders_spec.rb Show resolved Hide resolved

luisramos0 mentioned this pull request Feb 7, 2019

[Spree Upgrade] Fix adjustments sections in admin order edit page #3445

Merged

sauloperez reviewed Feb 7, 2019

View reviewed changes

Improve readability in admin/orders_spec

fe0fb22

mkllnk approved these changes Feb 7, 2019

View reviewed changes

spec/features/admin/orders_spec.rb Show resolved Hide resolved

sauloperez merged commit 92dca0c into openfoodfoundation:2-0-stable Feb 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spree Upgrade] Fix admin new order page #3194

[Spree Upgrade] Fix admin new order page #3194

HugsDaniel commented Dec 7, 2018 •

edited by luisramos0

Loading

luisramos0 left a comment •

edited

Loading

luisramos0 Dec 7, 2018

mkllnk commented Dec 7, 2018

HugsDaniel commented Dec 7, 2018

mkllnk commented Dec 7, 2018

mkllnk commented Dec 7, 2018

mkllnk commented Dec 7, 2018

HugsDaniel commented Dec 7, 2018

mkllnk commented Dec 10, 2018

luisramos0 commented Jan 8, 2019

luisramos0 commented Jan 31, 2019

luisramos0 commented Jan 31, 2019 •

edited

Loading

mkllnk left a comment

sauloperez Feb 7, 2019 •

edited

Loading

luisramos0 Feb 7, 2019 •

edited

Loading

sauloperez Feb 7, 2019

luisramos0 commented Feb 12, 2019

[Spree Upgrade] Fix admin new order page #3194

[Spree Upgrade] Fix admin new order page #3194

Conversation

HugsDaniel commented Dec 7, 2018 • edited by luisramos0 Loading

What? Why?

luisramos0 left a comment • edited Loading

Choose a reason for hiding this comment

luisramos0 Dec 7, 2018

Choose a reason for hiding this comment

mkllnk commented Dec 7, 2018

HugsDaniel commented Dec 7, 2018

mkllnk commented Dec 7, 2018

mkllnk commented Dec 7, 2018

mkllnk commented Dec 7, 2018

HugsDaniel commented Dec 7, 2018

mkllnk commented Dec 10, 2018

luisramos0 commented Jan 8, 2019

luisramos0 commented Jan 31, 2019

luisramos0 commented Jan 31, 2019 • edited Loading

mkllnk left a comment

Choose a reason for hiding this comment

sauloperez Feb 7, 2019 • edited Loading

Choose a reason for hiding this comment

luisramos0 Feb 7, 2019 • edited Loading

Choose a reason for hiding this comment

sauloperez Feb 7, 2019

Choose a reason for hiding this comment

luisramos0 commented Feb 12, 2019

HugsDaniel commented Dec 7, 2018 •

edited by luisramos0

Loading

luisramos0 left a comment •

edited

Loading

luisramos0 commented Jan 31, 2019 •

edited

Loading

sauloperez Feb 7, 2019 •

edited

Loading

luisramos0 Feb 7, 2019 •

edited

Loading