Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

Order completion #519

Merged
merged 9 commits into from
Jun 20, 2017
Merged

Order completion #519

merged 9 commits into from
Jun 20, 2017

Conversation

rmisio
Copy link
Contributor

@rmisio rmisio commented Jun 15, 2017

This PR adds in the ability to complete an order and leave a review.

.ratingStrip {
.ratingIcons {
display: inline-flex;
flex-direction: row-reverse;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wish we had a previous sibling selector in CSS. Instead, I'm fudging it with reversing the order in flex.

Copy link
Contributor

@jjeffryes jjeffryes left a comment

Choose a reason for hiding this comment

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

Looks good, I saw one place that might be duplicate code, and there are two console logs still in there, if this isn't the last PR for this section then they aren't an issue.

};

this.rating.set(data);
this.rating.set(data, { validate: true });
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does the data get set twice here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason is that I want to both validate the model and set with potentially invalid values. If you set with the validate option, the model will not set those attributes if any are invalid. But.. since I'm re-rendering the errors, I want them in the model.

Normally, this can be avoided by using Model.save(), which does set the attributes and will also validate, but in this case I don't want that because Save will kick off a request to the server if there are no errors and the request to the server will happen in the utility function.

@jjeffryes jjeffryes merged commit a6a85b8 into master Jun 20, 2017
@jjeffryes jjeffryes deleted the order-details-complete branch June 20, 2017 19:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants