Feature/salary calculator #156
Conversation
…nto feature/salary-calculator
|
In order to pass the tests you need the files to be ok with flake8, you can choose flake8 as a linter, and it will show you where are your mistakes. |
|
All linting errors, code conflicts and failing tests have been fixed and resolved. |
| from typing import Union | ||
|
|
||
|
|
||
| MINIMUM_WAGE = 29.12 |
There was a problem hiding this comment.
Can you please move this to setting-per-user basis?
There was a problem hiding this comment.
Not sure I understand your intention. Theses are the default values displayed to the user for settings creation (and also default values for the db model) but are editable by the user and saved on the db.
app/routers/salary/routes.py
Outdated
|
|
||
| @router.post('/view/{category_id}') | ||
| @router.get('/view/{category_id}') | ||
| async def view_salary(request: Request, category_id: int) -> Response: |
There was a problem hiding this comment.
Can you please split this function into multiple functions?
app/routers/salary/routes.py
Outdated
|
|
||
| @router.post('/edit') | ||
| @router.get('/edit') | ||
| async def pick_settings(request: Request) -> Response: |
There was a problem hiding this comment.
I have attempted to split all path operations into separate function, but after doing so the app failed to work properly and redirections caused disconnections rather than the wanted results. I have saved the new file and will look into it some more. Please let me know if you know why this might happen. (I haven't made any changes to the logic of the code besides splitting it to separate functions.)
Codecov Report
@@ Coverage Diff @@
## develop #156 +/- ##
===========================================
+ Coverage 99.29% 99.36% +0.07%
===========================================
Files 41 44 +3
Lines 1691 2059 +368
===========================================
+ Hits 1679 2046 +367
- Misses 12 13 +1
Continue to review full report at Codecov.
|
| settings = session.query(SalarySettings).filter(and_( | ||
| SalarySettings.user_id == user_id, SalarySettings.category_id | ||
| == category_id)).first() | ||
| session.close() |
There was a problem hiding this comment.
I can't really say why, but for some reason removing this line fails one of my tests.
The functionality is essentially the same, and the service itself doesn't seem to be affected, yet I didn't manage to pass the tests without it.
If you want to take a look at it, the one that fails is "test_routes: :test_edit_settings".
There was a problem hiding this comment.
Try to git checkout upstream/develop -- requirements.txt and pip install -U -r requirements.txt.
We've downgraded the pytest-asyncio library, that's might solve the problem
There was a problem hiding this comment.
Unfortunately this did not solve the problem.
|
While trying to solve conflicts, I've seen there is a minor problem with test_events.py. Can you please take a look? :) |
There was a missing import, now fixed. |
New feature - Salary Calculator:
allows users to select any category they previously created and create salary calculation settings for it. the feature also adds edit pages for the settings and view pages for the calculations. The calculation will be done for the chosen month and will be based on all events that are labeled with the corresponding category. Multiple salary settings are allowed, but only 1 for each user and category combination.
Further changes implemented:
-moved contents of file "app.database.database" to "app.database" for easier access.
-moved function "app.database,database.get_db" to "app.dependencies.get_db".
-merged file "tests.utils" into "app.internal.utils" to prevent code duplication.
-fixed imports and type hints where necessary.