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

Replace Spree.routes with Spree.pathFor #3605

Merged
merged 1 commit into from
May 26, 2020
Merged

Replace Spree.routes with Spree.pathFor #3605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .eslintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
"Sortable": "readonly",
"Spree": "readonly",
"Turbolinks": "readonly",
"update_state": "writable"
"update_state": "writable",
"Proxy": "readonly"
}
}
2 changes: 1 addition & 1 deletion backend/app/assets/javascripts/spree/backend/adjustments.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Spree.ready(function() {

Spree.ajax({
type: 'POST',
url: Spree.routes.apply_coupon_code(window.order_number),
url: Spree.pathFor('api/orders/' + window.order_number + '/coupon_codes'),
data: {
coupon_code: $("#coupon_code").val(),
token: Spree.api_key
Expand Down
2 changes: 1 addition & 1 deletion backend/app/assets/javascripts/spree/backend/admin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ Spree.ready(function(){
data: {
token: Spree.api_key
},
url: Spree.routes.checkouts_api + "/" + window.order_number + "/advance"
url: Spree.pathFor('api/checkouts/' + window.order_number + '/advance')
}).done(function() {
window.location.reload();
});
Expand Down
2 changes: 1 addition & 1 deletion backend/app/assets/javascripts/spree/backend/collections/states.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Spree.Collections.States = Backbone.Collection.extend({
},

url: function () {
return Spree.routes.states_search + "?country_id=" + this.country_id
return Spree.pathFor('api/states?country_id=' + this.country_id)
},

parse: function(resp, options) {
Expand Down
6 changes: 3 additions & 3 deletions backend/app/assets/javascripts/spree/backend/models/order.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//= require spree/backend/models/address

Spree.Models.Order = Backbone.Model.extend({
urlRoot: Spree.routes.orders_api,
urlRoot: Spree.pathFor('api/orders'),
idAttribute: "number",

relations: {
Expand All @@ -16,7 +16,7 @@ Spree.Models.Order = Backbone.Model.extend({

advance: function(opts) {
var options = {
url: Spree.routes.checkouts_api + "/" + this.id + "/advance",
url: Spree.pathFor('api/checkouts/' + this.id + '/advance'),
type: 'PUT',
};
_.extend(options, opts);
Expand All @@ -25,7 +25,7 @@ Spree.Models.Order = Backbone.Model.extend({

empty: function (opts) {
var options = {
url: Spree.routes.orders_api + "/" + this.id + "/empty",
url: Spree.pathFor('api/orders/' + this.id + '/empty'),
type: 'PUT',
};
_.extend(options, opts);
Expand Down
2 changes: 1 addition & 1 deletion backend/app/assets/javascripts/spree/backend/models/payment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Spree.Models.Payment = Backbone.Model.extend({
urlRoot: function() {
return Spree.routes.payments_api(this.get('order_id'));
return Spree.pathFor('api/orders/' + this.get('order_id') + '/payments')
}
});
2 changes: 1 addition & 1 deletion backend/app/assets/javascripts/spree/backend/models/shipment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Spree.Models.Shipment = Backbone.Model.extend({
idAttribute: "number",
paramRoot: "shipment",
urlRoot: Spree.routes.shipments_api,
urlRoot: Spree.pathFor('api/shipments'),

relations: {
"selected_shipping_rate": Backbone.Model,
Expand Down
2 changes: 1 addition & 1 deletion backend/app/assets/javascripts/spree/backend/models/stock_item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Spree.Models.StockItem = Backbone.Model.extend({
paramRoot: 'stock_item',
urlRoot: function() {
return Spree.routes.stock_items_api(this.get('stock_location_id'));
return Spree.pathFor('api/stock_locations/' + this.get('stock_location_id') + '/stock_items');
}
});
4 changes: 2 additions & 2 deletions backend/app/assets/javascripts/spree/backend/option_type_autocomplete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Spree.ready(function () {
multiple: true,
initSelection: function (element, callback) {
return Spree.ajax({
url: Spree.routes.option_type_search,
url: Spree.pathFor('api/option_types'),
data: { ids: element.val() },
type: 'get',
success: function(data) {
Expand All @@ -20,7 +20,7 @@ Spree.ready(function () {
});
},
ajax: {
url: Spree.routes.option_type_search,
url: Spree.pathFor('api/option_types'),
quietMillis: 200,
datatype: 'json',
params: { "headers": { 'Authorization': 'Bearer ' + Spree.api_key } },
Expand Down
4 changes: 2 additions & 2 deletions backend/app/assets/javascripts/spree/backend/option_value_picker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ $.fn.optionValueAutocomplete = function (options) {
minimumInputLength: 3,
multiple: multiple,
initSelection: function (element, callback) {
$.get(Spree.routes.option_value_search, {
$.get(Spree.pathFor('api/option_values'), {
ids: element.val().split(','),
token: Spree.api_key
}, function (data) {
callback(multiple ? data : data[0]);
});
},
ajax: {
url: Spree.routes.option_value_search,
url: Spree.pathFor('api/option_values'),
datatype: 'json',
data: function (term, page) {
// Note: This doesn't work. variants_product_id isn't an allowed filter
Expand Down
4 changes: 2 additions & 2 deletions backend/app/assets/javascripts/spree/backend/product_picker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ $.fn.productAutocomplete = function (options) {
minimumInputLength: 3,
multiple: multiple,
initSelection: function (element, callback) {
$.get(Spree.routes.admin_product_search, {
$.get(Spree.pathFor('admin/search/products'), {
ids: element.val().split(','),
token: Spree.api_key
}, function (data) {
callback(multiple ? data.products : data.products[0]);
});
},
ajax: {
url: Spree.routes.admin_product_search,
url: Spree.pathFor('admin/search/products'),
datatype: 'json',
params: { "headers": { 'Authorization': 'Bearer ' + Spree.api_key } },
data: function (term, page) {
Expand Down
61 changes: 40 additions & 21 deletions backend/app/assets/javascripts/spree/backend/routes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,46 @@
Spree.routes.checkouts_api = Spree.pathFor('api/checkouts')
Spree.routes.classifications_api = Spree.pathFor('api/classifications')
Spree.routes.option_value_search = Spree.pathFor('api/option_values')
Spree.routes.option_type_search = Spree.pathFor('api/option_types')
Spree.routes.orders_api = Spree.pathFor('api/orders')
Spree.routes.product_search = Spree.pathFor('api/products')
Spree.routes.admin_product_search = Spree.pathFor('admin/search/products')
Spree.routes.shipments_api = Spree.pathFor('api/shipments')
Spree.routes.checkouts_api = Spree.pathFor('api/checkouts')
Spree.routes.stock_locations_api = Spree.pathFor('api/stock_locations')
Spree.routes.taxon_products_api = Spree.pathFor('api/taxons/products')
Spree.routes.taxons_search = Spree.pathFor('api/taxons')
Spree.routes.user_search = Spree.pathFor('admin/search/users')
Spree.routes.variants_api = Spree.pathFor('api/variants')
Spree.routes.users_api = Spree.pathFor('api/users')
/**
* @deprecated Spree.routes will be removed in a future release. Please use Spree.pathFor instead.
* See: https://github.com/solidusio/solidus/issues/3405
*/

Spree.routes.line_items_api = function(order_id) {
return Spree.pathFor('api/orders/' + order_id + '/line_items')
var admin_routes = {
classifications_api: Spree.pathFor('api/classifications'),
option_value_search: Spree.pathFor('api/option_values'),
option_type_search: Spree.pathFor('api/option_types'),
orders_api: Spree.pathFor('api/orders'),
product_search: Spree.pathFor('api/products'),
admin_product_search: Spree.pathFor('admin/search/products'),
shipments_api: Spree.pathFor('api/shipments'),
checkouts_api: Spree.pathFor('api/checkouts'),
stock_locations_api: Spree.pathFor('api/stock_locations'),
taxon_products_api: Spree.pathFor('api/taxons/products'),
taxons_search: Spree.pathFor('api/taxons'),
user_search: Spree.pathFor('admin/search/users'),
variants_api: Spree.pathFor('api/variants'),
users_api: Spree.pathFor('api/users'),

line_items_api: function(order_id) {
return Spree.pathFor('api/orders/' + order_id + '/line_items')
},

payments_api: function(order_id) {
return Spree.pathFor('api/orders/' + order_id + '/payments')
},

stock_items_api: function(stock_location_id) {
return Spree.pathFor('api/stock_locations/' + stock_location_id + '/stock_items')
}
}

Spree.routes.payments_api = function(order_id) {
return Spree.pathFor('api/orders/' + order_id + '/payments')
var frontend_routes = {
states_search: Spree.pathFor('api/states'),
apply_coupon_code: function(order_id) {
return Spree.pathFor("api/orders/" + order_id + "/coupon_codes");
}
}

Spree.routes.stock_items_api = function(stock_location_id) {
return Spree.pathFor('api/stock_locations/' + stock_location_id + '/stock_items')
if(typeof Proxy == "function") {
Spree.routes = new Proxy(Object.assign(admin_routes, frontend_routes), Spree.routesDeprecationProxy);
} else {
Object.assign(Spree.routes, admin_routes)
}
10 changes: 5 additions & 5 deletions backend/app/assets/javascripts/spree/backend/shipments.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ var ShipShipmentView = Backbone.View.extend({
onSubmit: function(e){
Spree.ajax({
type: "PUT",
url: Spree.routes.shipments_api + "/" + this.shipment_number + "/ship",
url: Spree.pathFor('api/shipments/' + this.shipment_number + '/ship'),
data: {
send_mailer: this.$("[name='send_mailer']").is(":checked")
},
Expand All @@ -27,7 +27,7 @@ adjustShipmentItems = function(shipment_number, variant_id, quantity){
var shipment = _.findWhere(shipments, {number: shipment_number});
var inventory_units = _.where(shipment.inventory_units, {variant_id: variant_id});

var url = Spree.routes.shipments_api + "/" + shipment_number;
var url = Spree.pathFor('api/shipments/' + shipment_number);

var new_quantity = 0;
if(inventory_units.length<quantity){
Expand Down Expand Up @@ -106,15 +106,15 @@ var ShipmentSplitItemView = Backbone.View.extend({
split_attr.stock_location_id = target_id;
jqXHR = Spree.ajax({
type: "POST",
url: Spree.routes.shipments_api + "/transfer_to_location",
url: Spree.pathFor('api/shipments/transfer_to_location'),
data: split_attr
});
} else if (target_type == 'shipment') {
// transfer to an existing shipment
split_attr.target_shipment_number = target_id;
jqXHR = Spree.ajax({
type: "POST",
url: Spree.routes.shipments_api + "/transfer_to_shipment",
url: Spree.pathFor('api/shipments/transfer_to_shipment'),
data: split_attr
});
} else {
Expand Down Expand Up @@ -179,7 +179,7 @@ var ShipmentItemView = Backbone.View.extend({
var _this = this;
Spree.ajax({
type: "GET",
url: Spree.routes.variants_api + "/" + this.variant_id,
url: Spree.pathFor('api/variants/' + this.variant_id),
}).success(function(variant){
var split = new ShipmentSplitItemView({
shipmentItemView: _this,
Expand Down
4 changes: 2 additions & 2 deletions backend/app/assets/javascripts/spree/backend/taxon_autocomplete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ $.fn.taxonAutocomplete = function () {

Spree.ajax({
type: "GET",
url: Spree.routes.taxons_search,
url: Spree.pathFor('api/taxons'),
data: {
ids: ids,
per_page: count,
Expand All @@ -22,7 +22,7 @@ $.fn.taxonAutocomplete = function () {
});
},
ajax: {
url: Spree.routes.taxons_search,
url: Spree.pathFor('api/taxons'),
datatype: 'json',
data: function (term, page) {
return {
Expand Down
6 changes: 3 additions & 3 deletions backend/app/assets/javascripts/spree/backend/taxons.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Spree.ready(function() {
var saveSort = function(e) {
var item = e.item;
Spree.ajax({
url: Spree.routes.classifications_api,
url: Spree.pathFor('api/classifications'),
method: 'PUT',
data: {
product_id: item.getAttribute('data-product-id'),
Expand All @@ -27,7 +27,7 @@ Spree.ready(function() {
dropdownCssClass: "taxon_select_box",
placeholder: Spree.translations.find_a_taxon,
ajax: {
url: Spree.routes.taxons_search,
url: Spree.pathFor('api/taxons'),
params: {
"headers": {
'Authorization': 'Bearer ' + Spree.api_key
Expand Down Expand Up @@ -55,7 +55,7 @@ Spree.ready(function() {

$('#taxon_id').on("change", function(e) {
Spree.ajax({
url: Spree.routes.taxon_products_api,
url: Spree.pathFor('api/taxons/products'),
data: {
id: e.val,
simple: 1
Expand Down
4 changes: 2 additions & 2 deletions backend/app/assets/javascripts/spree/backend/user_picker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ $.fn.userAutocomplete = function () {
multiple: true,
initSelection: function (element, callback) {
Spree.ajax({
url: Spree.routes.users_api,
url: Spree.pathFor('api/users'),
data: {
ids: element.val()
},
Expand All @@ -20,7 +20,7 @@ $.fn.userAutocomplete = function () {
});
},
ajax: {
url: Spree.routes.users_api,
url: Spree.pathFor('api/users'),
datatype: 'json',
params: { "headers": { 'Authorization': 'Bearer ' + Spree.api_key } },
data: function (term) {
Expand Down
4 changes: 2 additions & 2 deletions backend/app/assets/javascripts/spree/backend/variant_autocomplete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@
minimumInputLength: 3,
initSelection: function(element, callback) {
Spree.ajax({
url: Spree.routes.variants_api + "/" + element.val(),
url: Spree.pathFor('api/variants/' + element.val()),
success: callback
});
},
ajax: {
url: Spree.routes.variants_api,
url: Spree.pathFor('api/variants'),
datatype: "json",
quietMillis: 500,
params: {
Expand Down
2 changes: 1 addition & 1 deletion backend/app/assets/javascripts/spree/backend/views/order/customer_select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Spree.Views.Order.CustomerSelect = Backbone.View.extend({
this.$el.select2({
placeholder: Spree.translations.choose_a_customer,
ajax: {
url: Spree.routes.users_api,
url: Spree.pathFor('api/users'),
params: { "headers": { 'Authorization': 'Bearer ' + Spree.api_key } },
datatype: 'json',
data: function(term, page) {
Expand Down
33 changes: 31 additions & 2 deletions core/app/assets/javascripts/spree.js.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,41 @@ Spree.ajax = function(url, options) {
return $.ajax(url, options);
};

Spree.routes = {
/**
* @deprecated Spree.routes will be removed in a future release. Please use Spree.pathFor instead.
* See: https://github.com/solidusio/solidus/issues/3405
*/
Spree.routesDeprecationProxy = {
get: function(obj, prop) {
<% if Rails.env != "production" %>
console.log("Spree.routes is deprecated, please use pathFor instead. See: https://github.com/solidusio/solidus/issues/3405");
<% end %>
Comment on lines +60 to +62
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to actually pass the environment to the JS code at runtime? I'm not a huge fan of mixing Ruby and JS like this, I think it could potentially generate a lot of confusion and subtle issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not quite sure how to go about this - do you have an example? We could assign a JS variable at the top of the file var production = <%= Rails.env == "production" %>, is that what you mean?

Also, I noticed in the frontend JS file, there's another deprecation done like this:

  if (console && console.warn) {
    console.warn('Spree.url is deprecated, and will be removed from a future Solidus version.');
  }

I hadn't noticed it before - would it be better to follow the already existing example set there?

Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

@seand7565 100%, I didn't realize we had done it before. This will print the warning even in production, but IMO it's okay to be consistent and follow the existing pattern. If we want to only print the warnings in production, let's make sure we change it everywhere, and maybe in a separate PR.

@kennyadsl what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Considering it only happens in the admin, I think it's not too bad if we print a deprecation warning in the console. So 👍 on making them more uniform.

For the other PR to use the Rais.env check, I guess the JS file in the frontend is not an .erb and I wouldn't add ruby to it since people can be including frontend files in unexpected ways (maybe with webpack?) and this would be a breaking change.


return obj[prop]
},

set: function(obj, prop, value) {
<% if Rails.env != "production" %>
console.log("Spree.routes is deprecated, please use pathFor instead. See: https://github.com/solidusio/solidus/issues/3405");
<% end %>

obj[prop] = value
return value;
}
};

var frontend_routes = {
states_search: Spree.pathFor('api/states'),
apply_coupon_code: function(order_id) {
return Spree.pathFor("api/orders/" + order_id + "/coupon_codes");
}
};
}

if(typeof Proxy == "function") {
Spree.routes = new Proxy(frontend_routes, Spree.routesDeprecationProxy);
} else {
Spree.routes = frontend_routes
}

Spree.getJSON = function(url, data, success) {
if (typeof data === 'function') {
Expand Down
2 changes: 1 addition & 1 deletion frontend/app/assets/javascripts/spree/frontend/checkout/address.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Spree.ready(function($) {
if (countryId != null) {
if (statesByCountry[countryId] == null) {
$.get(
Spree.routes.states_search,
Spree.pathFor('api/states'),
{
country_id: countryId
},
Expand Down
Loading