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

Speed up the products table publication #5128

Merged
merged 3 commits into from
Apr 17, 2019
Merged

Speed up the products table publication #5128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
439fa92
feat: faster products publication for operator UI
aldeed Apr 16, 2019
c31e29e
fix: money formatting issues
aldeed Apr 16, 2019
f28a19f
chore: lint fix
aldeed Apr 17, 2019
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
123 changes: 34 additions & 89 deletions client/modules/i18n/currency.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import accounting from "accounting-js";
import { Reaction, Logger } from "/client/api";
import ReactionError from "@reactioncommerce/reaction-error";
import { formatMoney } from "accounting-js";
import { Reaction } from "/client/api";
import { Shops, Accounts } from "/lib/collections";
import { currencyDep } from "./main";

Expand Down Expand Up @@ -48,17 +47,9 @@ export function findCurrency(defaultCurrency, useDefaultShopCurrency) {
* @memberof i18n
* @method
* @param {String} formatPrice - currentPrice or "xx.xx - xx.xx" formatted String
* @param {Boolean} useDefaultShopCurrency - flag for displaying shop's currency in Admin view of PDP
* @return {String} returns locale formatted and exchange rate converted values
*/
export function formatPriceString(formatPrice, useDefaultShopCurrency) {
let defaultShopCurrency = useDefaultShopCurrency;

// in case useDefaultShopCurrency is a Spacebars.kw we have this check
if (typeof useDefaultShopCurrency === "object" || !useDefaultShopCurrency) {
defaultShopCurrency = false;
}

export function formatPriceString(formatPrice) {
currencyDep.depend();
const locale = Reaction.Locale.get();

Expand All @@ -72,93 +63,47 @@ export function formatPriceString(formatPrice, useDefaultShopCurrency) {
}

// get user currency instead of locale currency
const userCurrency = findCurrency(locale.currency, defaultShopCurrency);
const userCurrency = findCurrency(locale.currency, true);

// for the cases then we have only one price. It is a number.
const currentPrice = formatPrice.toString();
let price = 0;
const prices = currentPrice.indexOf(" - ") >= 0 ?
currentPrice.split(" - ") : [currentPrice];

// basic "for" is faster then "for ...of" for arrays. We need more speed here
const len = prices.length;
for (let i = 0; i < len; i += 1) {
const originalPrice = prices[i];
try {
// we know the locale, but we don"t know exchange rate. In that case we
// should return to default shop currency
if (typeof userCurrency.rate !== "number") {
throw new ReactionError("invalid-exchange-rate", "Exchange rate is invalid");
}
// Only convert for non-admin view.
if (!defaultShopCurrency) {
prices[i] *= userCurrency.rate;
}
currentPrice.split(" - ") : [currentPrice, currentPrice];

price = _formatPrice(
price, originalPrice, prices[i],
currentPrice, userCurrency, i, len
);
} catch (error) {
Logger.debug("currency error, fallback to shop currency");
price = _formatPrice(
price, originalPrice, prices[i],
currentPrice, locale.shopCurrency, i, len
);
}
}
return price;
return getDisplayPrice(Number(prices[0]), Number(prices[1]), userCurrency);
}

/**
* _formatPrice
* private function for formatting locale currency
* @private
* @param {Number} price price
* @param {Number} originalPrice originalPrice
* @param {Number} actualPrice actualPrice
* @param {Number} currentPrice currentPrice
* @param {Number} currency currency
* @param {Number} pos position
* @param {Number} len length
* @return {Number} formatted price
* @name getDisplayPrice
* @method
* @summary Returns a price for front-end display in the given currency
* @param {Number} minPrice Minimum price
* @param {Number} maxPrice Maximum price
* @param {Object} currencyInfo Currency object from Reaction shop schema
* @returns {String} Display price with currency symbol(s)
*/
function _formatPrice(
price, originalPrice, actualPrice, currentPrice, currency,
pos, len
) {
// this checking for locale.shopCurrency mostly
if (typeof currency !== "object") {
return false;
}

let adjustedPrice = actualPrice;
let formattedPrice;

// Precision is mis-used in accounting js. Scale is the propery term for number
// of decimal places. Let's adjust it here so accounting.js does not break.
if (currency.scale !== undefined) {
currency.precision = currency.scale;
}

// If there are no decimal places, in the case of the Japanese Yen, we adjust it here.
if (currency.scale === 0) {
adjustedPrice = actualPrice * 100;
}
function getDisplayPrice(minPrice, maxPrice, currencyInfo = { symbol: "" }) {
let displayPrice;

// @param {string} currency.where: If it presents - in situation then two
// prices in string, currency sign will be placed just outside the right price.
// For now it should be manually added to fixtures shop data.
if (typeof currency.where === "string" && currency.where === "right" &&
len > 1 && pos === 0) {
const modifiedCurrency = Object.assign({}, currency, {
symbol: ""
});
formattedPrice = accounting.formatMoney(adjustedPrice, modifiedCurrency);
if (minPrice === maxPrice) {
// Display 1 price (min = max)
displayPrice = formatMoney(minPrice, currencyInfo);
} else {
// accounting api: http://openexchangerates.github.io/accounting.js/
formattedPrice = accounting.formatMoney(adjustedPrice, currency);
// Display range
let minFormatted;

// Account for currencies where only one currency symbol should be displayed. Ex: 680,18 - 1 359,68 руб.
if (currencyInfo.where === "right") {
const modifiedCurrencyInfo = Object.assign({}, currencyInfo, {
symbol: ""
});
minFormatted = formatMoney(minPrice, modifiedCurrencyInfo).trim();
} else {
minFormatted = formatMoney(minPrice, currencyInfo);
}

const maxFormatted = formatMoney(maxPrice, currencyInfo);
displayPrice = `${minFormatted} - ${maxFormatted}`;
}

return price === 0 ? currentPrice.replace(originalPrice, formattedPrice) : price.replace(originalPrice, formattedPrice);
return displayPrice;
}
32 changes: 8 additions & 24 deletions imports/plugins/core/core/server/publications/collections/products.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,33 +392,17 @@ Meteor.publish("ProductsAdminList", function (page = 0, limit = 24, productFilte

delete selector.isVisible; // in edit mode, you should see all products

Counts.publish(this, "products-count", Products.find(selector));
// noReady and nonReactive are needed for good performance on
// large data sets
Counts.publish(this, "products-count", Products.find(selector), {
noReady: true,
nonReactive: true
});

// Get the IDs of the first N (limit) top-level products that match the query
const productIds = Products.find(selector, {
// Get the first N (limit) top-level products that match the query
return Products.find(selector, {
sort,
skip: page * limit,
limit
}, {
fields: {
_id: 1
}
}).map((product) => product._id);

// Return a cursor for the matching products plus all their variants
return Products.find({
$or: [{
ancestors: {
$in: productIds
}
}, {
_id: {
$in: productIds
}
}]
}, {
sort
// We shouldn't limit here. Otherwise we are limited to 24 total products which
// could be far less than 24 top-level products.
});
});
48 changes: 2 additions & 46 deletions lib/api/catalog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import _ from "lodash";
import { Products } from "/lib/collections";

/**
Expand All @@ -19,51 +18,8 @@ export default {
* @return {Object} range, min, max
*/
getProductPriceRange(productId) {
const product = Products.findOne(productId);
if (!product) {
return {
range: "0",
min: 0,
max: 0
};
}

const variants = this.getTopVariants(product._id);
// if we have variants we have a price range.
// this processing will default on the server
const visibileVariant = variants.filter((variant) => variant.isVisible === true);

if (visibileVariant.length > 0) {
const variantPrices = [];
variants.forEach((variant) => {
if (variant.isVisible === true) {
const range = this.getVariantPriceRange(variant._id);
if (typeof range === "string") {
const firstPrice = parseFloat(range.substr(0, range.indexOf(" ")));
const lastPrice = parseFloat(range.substr(range.lastIndexOf(" ") + 1));
variantPrices.push(firstPrice, lastPrice);
} else {
variantPrices.push(range);
}
} else {
variantPrices.push(0, 0);
}
});
const priceMin = _.min(variantPrices);
const priceMax = _.max(variantPrices);
let priceRange = `${priceMin.toFixed(2)} - ${priceMax.toFixed(2)}`;
// if we don't have a range
if (priceMin === priceMax) {
priceRange = priceMin.toFixed(2);
}
return {
range: priceRange,
min: priceMin,
max: priceMax
};
}

if (!product.price) {
const product = Products.findOne({ _id: productId });
if (!product || !product.price) {
return {
range: "0",
min: 0,
Expand Down