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.

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

Cart in socket #44

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Cart in socket #44

wants to merge 17 commits into from

Conversation

MICHAELMUNAVU83
Copy link
Collaborator

@MICHAELMUNAVU83 MICHAELMUNAVU83 commented Nov 15, 2023

Notes for the reviewer

In this PR, we are adding ticket types to the cart via the socket . I have added tests for the cart context and the /home page .

GIF (for features)

Screen.Recording.2023-11-16.at.07.53.02.mov

Review checklist

  • I have performed a self-review of my code
  • Tests are passing.
  • My CI is passing

def add_to_cart(cart, cart_item_id) do
cart_list_ids = cart_list_ids(cart)

case Enum.member?(cart_list_ids, cart_item_id) do
Copy link
Collaborator

Choose a reason for hiding this comment

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

A Case statement is overkill here. You can just use an if

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True , since it is either true or false

Copy link
Member

Choose a reason for hiding this comment

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

You need to introduce styler, it auto corrects such https://github.com/adobe/elixir-styler

case Enum.member?(cart_list_ids, cart_item_id) do
true ->
ticket_type = Enum.find(cart, fn item -> item.id == cart_item_id end)
updated_cart = get_updated_cart("in-cart", ticket_type, cart)
Copy link
Collaborator

Choose a reason for hiding this comment

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

you don't need to have a variable here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks , I have made this change


false ->
ticket_type = TicketTypes.get_ticket_type!(cart_item_id)
updated_cart = get_updated_cart("out-of-cart", ticket_type, cart)
Copy link
Collaborator

Choose a reason for hiding this comment

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

you don't need this variable. The return value of the function is enough

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks , I have fixed this

def handle_event("add_to_cart", %{"id" => id}, socket) do
cart_item_ids = Cart.cart_list_ids(socket.assigns.cart)

if Enum.member?(cart_item_ids, String.to_integer(id)) do
Copy link
Collaborator

Choose a reason for hiding this comment

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

whats happening here? It seems whether this is true or false the item is still added to the cart and only the flash message is different

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How @okothkongo suggested we handle this is such that , if there is an item in the cart , and someone adds the same item to the cart , it updates the quantity of that item in the cart and updates the user that the quantity has been added as opposed to disabling the add to cart for an item already in the cart

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