Skip to content

ProductRetrieveView:param:with_direct_siblings if True return product_pre, product_next to the context and direct_siblings in serializer#801

Open
haricot wants to merge 10 commits into
awesto:releases/1.2from
haricot:github/one_product_or_pre_cur_nex
Open

ProductRetrieveView:param:with_direct_siblings if True return product_pre, product_next to the context and direct_siblings in serializer#801
haricot wants to merge 10 commits into
awesto:releases/1.2from
haricot:github/one_product_or_pre_cur_nex

Conversation

@haricot

@haricot haricot commented Apr 7, 2020

Copy link
Copy Markdown
Contributor

Same things as the linked pull request #734 but reviewed in a simpler way.

@haricot

haricot commented Apr 7, 2020

Copy link
Copy Markdown
Contributor Author

Otherwise change ProductRetrieveView.as_view in myshop.cms_app.CatalogListApp should be sufficient and in this case this pull request can be closed.

@jrief

jrief commented Apr 7, 2020

Copy link
Copy Markdown
Member

Thanks for this pull request. Rendering the previous and next siblings of a product, may really be useful in some situations.
Just some points I would have done differently:

  • Instead of using a tag named one_product, I would use something such as with_direct_siblings.
  • Don't change the semantics of the method get_object(), this should always return one and only one product. Instead add another method named say get_object_with_direct_siblings() .

But in general looking at your code, how does this view function behave, if someone filters by a certain product tag, or uses the search input field? Where is that information kept while rendering the product's detail view?

For this we have three possible solutions. Add this filtering information to the session, or to a cookie or as a request parameter. The latter generates ugly URLs for the detail view. Session could be problematic, because in django-SHOP the session_id is generated only after adding the first product to the cart, or as logged in user. So the remaining possibility is a cookie.

Let's discuss this on a chat.

@haricot haricot changed the title ProductRetrieveView:param:one_product if False return pre,cur,nex products to the context ProductRetrieveView:param:with_direct_siblings if True return pre,cur,nex products to the context Apr 8, 2020
@haricot
haricot force-pushed the github/one_product_or_pre_cur_nex branch from e851bff to 02ee0cc Compare April 9, 2020 12:35
@haricot haricot changed the title ProductRetrieveView:param:with_direct_siblings if True return pre,cur,nex products to the context ProductRetrieveView:param:with_direct_siblings if True return pre,nex products to the context and direct_siblings Apr 9, 2020
@haricot haricot changed the title ProductRetrieveView:param:with_direct_siblings if True return pre,nex products to the context and direct_siblings ProductRetrieveView:param:with_direct_siblings if True return pre,nex products to the context and direct_siblings in serializer Apr 9, 2020
@haricot haricot changed the title ProductRetrieveView:param:with_direct_siblings if True return pre,nex products to the context and direct_siblings in serializer ProductRetrieveView:param:with_direct_siblings if True return product_pre,product_next to the context and direct_siblings in serializer Apr 9, 2020
@haricot haricot changed the title ProductRetrieveView:param:with_direct_siblings if True return product_pre,product_next to the context and direct_siblings in serializer ProductRetrieveView:param:with_direct_siblings if True return product_pre, product_next to the context and direct_siblings in serializer Apr 9, 2020
@haricot
haricot force-pushed the github/one_product_or_pre_cur_nex branch from c22018a to 2bdfc8b Compare April 10, 2020 15:22
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.

2 participants