Code Review : Could promises be 'chained'?
I am developing a simple Quote tool for the business-owner to fill in a form (item number, item name, price, etc) and send to the potential customer.
This code review request is in reference to the "view-quote" page, which simply displays a quote that was previously sent. (see pic)
Data Structure
As you can see, the data is broken up into three tables:
- Quotes Table: Holds general info about the quote.
- Clients Table: Holds general client info.
- Items Table: Holds individual quote items.
Fetch-ing The Quote Data
When a user hits this page, a parameter is passed in the URL (for example):
view-quote.html?quote_id=GL555
So from here, I needed to query the QUOTES table first.
Within those results, is our CLIENT_ID. Using that, I then query the CLIENTS table.
Lastly, I query the ITEMS table to get all items associated with the QUOTE.
Did I Break A Promise?
I am fairly new to writing modern JavaScript, however, I did manage to get this code functioning. But as you will see, I believe this code could be re-factored a number of ways.
The JavaScript
// Quotes View - View Model
// +---------------------------------------------------------------------------+
// | View Quote View Model |
// | |
// | view-quote-view-model.js |
// +---------------------------------------------------------------------------+
// | |
// +---------------------------------------------------------------------------+/
let ItemsModel = function (item_number, item_name, item_price, quantity, manufacturer, subtotal) {
this.item_number = item_number;
this.item_name = item_name;
this.item_price = item_price;
this.quantity = quantity;
this.manufacturer = manufacturer;
this.subtotal = subtotal;
}
// GLOBAL PAGE VARIABLES
var gQuoteNumber = $_GET("quote_number");
var gClientId;
var gGrandTotal = 0;
console.log(`Quote Number: ${gQuoteNumber}`);
function ViewQuoteViewModel() {
var self = this; // Scope Trick
/* QUOTE Observables */
self.quote_id = ko.observable();
self.quote_number = ko.observable(gQuoteNumber);
self.quote_date = ko.observable();
self.amount = ko.observable();
self.business_name = ko.observable();
self.notes = ko.observable();
/* CLIENT Observables */
self.client_id = ko.observable();
self.first_name = ko.observable();
self.last_name = ko.observable();
self.email = ko.observable();
self.phone = ko.observable();
self.address = ko.observable();
self.city = ko.observable();
self.state = ko.observable();
self.zip = ko.observable();
self.grand_total = ko.observable(gGrandTotal);
/* ITEMS Observables */
self.items = ko.observableArray();
/* COMPUTED */
self.fullName = ko.computed(function() {
return self.first_name() + " " + self.last_name();
}, self);
/* GET PAGE DATA */
self.getQuote = function (quoteNumber) {
return fetch(apiQuotesOne + gQuoteNumber, {
method: "GET",
headers: {
"Content-Type": "application/json"
}
})
.then(handleErrors)
.then(function(response) {
return response.json();
}).then(function(data) {
// Populate Observables
self.quote_id(data[0].id);
self.quote_number(gQuoteNumber);
self.quote_date(data[0].quote_date);
self.amount(data[0].amount);
self.business_name(data[0].business_name);
self.notes(data[0].notes);
self.client_id(data[0].client_id)
// Set Global Variable
gClientId = data[0].client_id;
// GO TO NEXT FUNCTION, GET CLIENT INFORMATION
self.getClient(gClientId);
});
};
self.getClient = function(clientId){
return fetch(apiCustomersOne + gClientId, {
method: "GET",
headers: {
"Content-Type": "application/json"
}
})
.then(handleErrors)
.then(function(response) {
return response.json();
}).then(function(data) {
// Populate Observables
console.log(data);
self.first_name(data[0].first_name);
self.last_name(data[0].last_name);
self.email(data[0].email);
self.phone(data[0].phone);
self.address(data[0].address);
self.city(data[0].city);
self.state(data[0].state)
self.zip(data[0].zip)
// GO TO NEXT FUNCTION, GET ITEMS
self.getItems();
});
}
self.getItems = function(quoteId){
return fetch(apiItems + gQuoteNumber, {
method: "GET",
headers: {
"Content-Type": "application/json"
}
})
.then(handleErrors)
.then(function(response) {
return response.json();
}).then(function(data) {
// Populate Items Array
var subtotal;
$.each(data,
function (key, val) {
//console.log(val);
subtotal = val.quantity * val.item_price;
gGrandTotal = gGrandTotal + subtotal;
self.grand_total(gGrandTotal);
self.items.push(new ItemsModel(val.item_number,val.item_name, val.item_price, val.quantity, val.manufacturer, subtotal ));
});
});
}
// Kick it off
self.getQuote(gQuoteNumber);
}
ko.applyBindings(new ViewQuoteViewModel());
...here is my helper function to grab query string parameters:
function $_GET(param) {
var vars = {};
window.location.href.replace( location.hash, '' ).replace(
/[?&]+([^=&]+)=?([^&]*)?/gi, // regexp
function( m, key, value ) { // callback
vars[key] = value !== undefined ? value : '';
}
);
if ( param ) {
return vars[param] ? vars[param] : null;
}
return vars;
}
I got this working by calling one function from inside another. It just feels like "cheating". It feels like bad code.
Could anyone point me in the right direction to clean this up a bit? I am thinking the three calls to the API can be really chained.
Thank you for looking.
PS. I am using KNOCKOUTJS as my MVVM pattern, in case that changes anything.
John