Feature/weekly tasks #146
Feature/weekly tasks #146
Conversation
…rs and basic functions for- edit, remove and add + demo functions for demo operations. added basic pages - add_weekly_task.html, weekly_tasks_manager.html
…ted all the demo functions. Arrange the code to be clearer and shorter.
…ode. and added wekly_tasks.py on the intertnal directory
… router, temporary
…nction and where it needs to be placed
Codecov Report
@@ Coverage Diff @@
## develop #146 +/- ##
===========================================
+ Coverage 95.81% 96.01% +0.20%
===========================================
Files 75 77 +2
Lines 3343 3515 +172
===========================================
+ Hits 3203 3375 +172
Misses 140 140
Continue to review full report at Codecov.
|
|
Great work! I've added few of my insights :) |
| @@ -59,6 +59,20 @@ def profile_test_client(): | |||
| Base.metadata.drop_all(bind=test_engine) | |||
|
|
|||
|
|
|||
| @pytest.fixture(scope="session") | |||
| def weekly_tasks_test_client(): | |||
There was a problem hiding this comment.
Great! Maybe we should use it in more tests?
There was a problem hiding this comment.
the other functions (internal) are not client related functions
so it seems I can't use it on them.
There was a problem hiding this comment.
it seems the comment about the 'Depends(get_db)' encounters the same problem
the Depends is of no use if the function is not a router function.
I did not find a way to use it within a standard non-fastapi function
the internal functions are used by the routers functions which uses Depends
There was a problem hiding this comment.
I split the tests into two files: internal tests and route tests.
Tell me what you think about the rearrangement and my comments :)
app/routers/weekly_tasks.py
Outdated
| massage = None | ||
| if mode == "add": | ||
| massage = "could not add The Weekly Task" | ||
| made_change = create_weekly_task( |
There was a problem hiding this comment.
different functions, in the next commits I will change the names and variables to make it clearer
There was a problem hiding this comment.
changed the names so it will be more clear, please look again, tell me what you think :)
app/routers/weekly_tasks.py
Outdated
| weekly_task_id=weekly_task_id | ||
| ) | ||
|
|
||
| made_change = False |
There was a problem hiding this comment.
changed the names so it will be more clear, please look again, tell me what you think :)
|
Great work, but we still have a little way to go :) |
app/database/models.py
Outdated
| content = Column(String) | ||
| is_done = Column(Boolean, nullable=False) | ||
| is_important = Column(Boolean, nullable=False) | ||
| date_time = Column(DateTime, nullable=False) |
There was a problem hiding this comment.
Try to give more specific variable names.
is it the creation date, or maybe the deadline, it is hard to tell.
There was a problem hiding this comment.
I should not mess with this model, its not mine
| day = day_full_name[:3] | ||
| day_lower = day.lower() | ||
| if day in days_list: | ||
| checked_days.append((day_full_name, day_lower, "checked")) | ||
| else: | ||
| checked_days.append((day_full_name, day_lower, "")) |
There was a problem hiding this comment.
I did not understand
| ) | ||
|
|
||
|
|
||
| @router.get("/add", dependencies=[Depends(is_logged_in)]) |
There was a problem hiding this comment.
Add is_logged_in to the router's dependencies instead
There was a problem hiding this comment.
I realized that the function is meant to be used in this way as well.
In case I do not need to use the variable it return's inside the router.
This reverts commit dab05ee.
Added tables, routers, tests, templates, and internal functions
The fitter gives the option to set weekly tasks.
Comes with a page for managing weekly tasks and another page for editing the task.
Gives the ability to edit, add and delete a weekly task.
Along with this in the feature is included a function for creating the tasks - which aims to create tasks every week for the user (according to the definition of his weekly tasks)