Skip to content

change the context_processor in the shopping cart to return a pointer to... - #6242

Merged
chrisndodge merged 1 commit into
masterfrom
cdodge/move-shoppingcart-button-detection
Dec 14, 2014
Merged

change the context_processor in the shopping cart to return a pointer to...#6242
chrisndodge merged 1 commit into
masterfrom
cdodge/move-shoppingcart-button-detection

Conversation

@chrisndodge

Copy link
Copy Markdown
Contributor

... a function so that only the templates that actually need to know if the shopping cart button should be shown will actually make the database roundtrips

@chrisndodge

Copy link
Copy Markdown
Contributor Author

@jbau @cpennington @andy-armstrong

This will fix the performance issue regarding shopping cart Mako context processing.

@jbau since I was in that area of the code, I opportunistically implemented an "does shoppingcart exist" check for us to be able to stop creating empty shoppingcarts when people simply visit the site. This is something we talked about in the past, so I figured I'd just go for it while I was in here :-)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I had trouble reading this until I realized that the function doesn't actually display the shopping cart. I think it would be clearer if it was named something like 'should_display_shopping_cart_fun' or 'has_shopping_cart_func' or something like this that implies that it will return a boolean.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This function is also missing a doc string which is why the quality build failed.

@andy-armstrong

Copy link
Copy Markdown
Contributor

👍 Thanks for tackling this so quickly, @chrisndodge.

@chrisndodge
chrisndodge force-pushed the cdodge/move-shoppingcart-button-detection branch from 4fcae3b to b168087 Compare December 14, 2014 15:25
… to a function so that only the templates that actually need to know if the shopping cart button should be shown will actually make the database roundtrips
@chrisndodge
chrisndodge force-pushed the cdodge/move-shoppingcart-button-detection branch from b168087 to 1148366 Compare December 14, 2014 16:34
@chrisndodge

Copy link
Copy Markdown
Contributor Author

@andy-armstrong renamed methods to be more descriptive. Need to rebuild due to a method name being too long :-(

@jbau

jbau commented Dec 14, 2014

Copy link
Copy Markdown

👍

chrisndodge pushed a commit that referenced this pull request Dec 14, 2014
…ection

change the context_processor in the shopping cart to return a pointer to...
@chrisndodge
chrisndodge merged commit 8a685ce into master Dec 14, 2014
@benpatterson
benpatterson deleted the cdodge/move-shoppingcart-button-detection branch January 7, 2015 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants