Creating orders.html, presenting orders history #95

OfirMatasas · 2022-04-28T20:12:01Z

A page on Hotails, where the user can watch the history of all of his
orders, cancel the orders which havn't started yet, and approve orders
as a daycare.

Signed-off-by: Ofir Matasas [email protected]

Desired Outcome

A page on Hotails, where the user can watch the history of all of his orders, cancel the orders which haven't started yet, and approve orders as a daycare.

Implemented Changes

Create orders.html + orders.css files
Added logic in getting to the orders page and presenting orders information (base_template navbar, main.urls, orders.views)
added 3 methods in Orders:
1. Check if the order is approvable
2. Check if the order is cancelable
3. Get the status of the order

Connected Issue/Story

Closes #94

Definition of Done

All the options mentioned in the desired outcome are available to the users.

Picture of the orders page

ErezCohenn

Great job @OfirMatasas!
take a look at the comment I left.

main/views.py

orders/models.py

orders/templates/orders/orders.html

main/tests.py

orders/models.py

Yuval-Vino

Great job ofir! orders HTML looks good , I left 1 comment tell me what you think

main/tests.py

Capacity dates check for show only available daycares in dates range. "get_capacity_of_daycare_in_dates_range" function took from redhat-beyond#95 . waiting for approval. Signed-off-by: tamirmatok <[email protected]>

tamirmatok

Great job @OfirMatasas !!
I left some comments let me know what u think.

main/tests.py

orders/models.py

main/tests.py

Capacity dates check for show only available daycares in dates range. "get_capacity_of_daycare_in_dates_range" function took from #95 . waiting for approval. Signed-off-by: tamirmatok <[email protected]>

main/views.py

main/tests.py

orders/models.py

Omeramsc · 2022-05-11T15:56:13Z

orders/tests.py

+        create_order.status = new_status
+        assert create_order.get_order_status() == expected_result
+
+    def test_get_daycare_capacity_in_dates_range(self, create_order):


That's not how you should test this function. it has a lot of logic; you need to test different cases and see that you get the capacity you expect in each case

I thought that checking the current capacity of a daycare before and after approving an order will check if the method is working correctly.
Also, some of the other tests are testing this method too (like "test_order_is_approvable_according_to_daycare_capacity").

Do you have any example for a case that I should test for this method? Or another way for testing it?

first of all - test names should follow the method they're testing (but not only the method name).
as an example:
if the method name is: check_capacity_of_daycare
possible test names are:
test_check_capacity_of_daycare
test_check_capacity_of_daycare_raise_value_error_when_something_happends
test_check_capacity_of_daycare_reduced_after_an_order
etc.

otherwise, it is very confusing to understand what is testing what.

second:
the test above test if order can be approved, according to its overall calculated capacity
this test should be a specific unit-test for get_capacity_of_daycare_in_dates_range it doesn't test the overall logic your app will go through, but it is testing this method specifically, and should pass even if in the future you'll decide to change the way the process works.

this method has a lot of cases. think of each if statement as at least two possible test-cases.

@OfirMatasas are you going to address these?

Omeramsc · 2022-05-11T15:56:47Z

orders/models.py

+
+        return capacity_per_day
+
+    def are_order_dates_available(self):


I made "test_order_is_approvable_according_to_daycare_capacity" for this method. Should I test it in another way?

Is it testing this function alone?
if it does, it deserves a name change :)

Otherwise, we need a test to test this only

The only thing this function does is check if specific dates are available due to capacity. the test you wrote deal with the whole process your application does. What we need for this function is a unit test, as it tests this unit specifically, without dependencies from other processes.

Capacity dates check for show only available daycares in dates range. "get_capacity_of_daycare_in_dates_range" function took from #95 . waiting for approval. Signed-off-by: tamirmatok <[email protected]>

Capacity dates check for show only available daycares in dates range. "get_capacity_of_daycare_in_dates_range" function took from redhat-beyond#95 . waiting for approval. Signed-off-by: tamirmatok <[email protected]>

A page on Hotails, where the user can watch the history of all of his orders, cancel the orders which havn't started yet, and approve orders as a daycare. Signed-off-by: Ofir Matasas <[email protected]>

ormergi

Hi Ofir, please see my inline comments

ormergi · 2022-05-25T06:09:35Z

orders/models.py

+    def get_order_status(self):
+        if self.status == StatusOptions.Pending:
+            return "Pending"
+        elif self.status == StatusOptions.Approved:
+            return "Approved"
+        elif self.status == StatusOptions.Canceled:
+            return "Canceled"
+        elif self.status == StatusOptions.OnGoing:
+            return "Ongoing"
+        return "Finished"


I think this can be simplified to

statusOptionsNameIdx=1 reuturn self.status[statusOptionsNameIdx]

WDYT?

ormergi · 2022-05-25T06:18:37Z

orders/models.py

+        return "Finished"
+
+    @staticmethod
+    def get_capacity_of_daycare_in_dates_range(daycare_id: DayCare, start_datetime: datetime, end_datetime: datetime):


What is capacity?

ormergi · 2022-05-25T06:20:31Z

orders/models.py

+        start_date = datetime.date(year=start_datetime.year, month=start_datetime.month, day=start_datetime.day)
+        end_date = datetime.date(year=end_datetime.year, month=end_datetime.month, day=end_datetime.day)
+        relevant_orders = Order.objects.filter(daycare_id=daycare_id,
+                                               status__in=['A', 'O'],


Is it possible to use full names?

ormergi · 2022-05-25T06:28:57Z

orders/tests.py

+        create_order.status = new_status
+        assert create_order.get_order_status() == expected_result
+
+    def test_get_daycare_capacity_in_dates_range(self, create_order):


@OfirMatasas are you going to address these?

ormergi · 2022-05-25T06:30:30Z

orders/views.py

+def orders(request):
+    if DogOwner.objects.filter(user=request.user).exists():
+        context = {
+            'orders': Order.objects.filter(dog_owner_id__user=request.user),


Why is it necessary to fillter by authenticated user, is it possible to use request.user directly?

Omeramsc · 2022-05-26T14:31:57Z

There are conflicts and a bunch of unresolved comments. Sorry, it won't be ready for the demo.

OfirMatasas added the Status/In-Progress label Apr 28, 2022

OfirMatasas self-assigned this Apr 28, 2022

OfirMatasas force-pushed the orders-page branch 4 times, most recently from a147283 to 650070c Compare April 29, 2022 09:22

OfirMatasas added Ready for review and removed Status/In-Progress labels Apr 29, 2022

OfirMatasas requested review from tamirmatok, Omeramsc, ormergi, ErezCohenn, jeniaSakirko and Yuval-Vino April 29, 2022 09:28

OfirMatasas force-pushed the orders-page branch 4 times, most recently from 9a0d554 to d36c343 Compare April 29, 2022 09:48

ErezCohenn requested changes Apr 29, 2022

View reviewed changes

main/views.py Outdated Show resolved Hide resolved

orders/models.py Outdated Show resolved Hide resolved

orders/models.py Outdated Show resolved Hide resolved

orders/templates/orders/orders.html Show resolved Hide resolved

OfirMatasas force-pushed the orders-page branch from d36c343 to b3b9cb5 Compare April 30, 2022 11:40

OfirMatasas requested a review from ErezCohenn April 30, 2022 13:30