-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: main
Are you sure you want to change the base?
Cart in socket #44
Conversation
…onfAfrica into cart-in-socket
…into cart-in-socket
lib/elixir_conf_africa/cart.ex
Outdated
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
lib/elixir_conf_africa/cart.ex
Outdated
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) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
lib/elixir_conf_africa/cart.ex
Outdated
|
||
false -> | ||
ticket_type = TicketTypes.get_ticket_type!(cart_item_id) | ||
updated_cart = get_updated_cart("out-of-cart", ticket_type, cart) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
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