Separate exercise list into current/previous exercises

[senier]

I chose the previous 31 days as the cutoff for current exercises, but have no strong opinion here...

[treiher]

That's a good idea. The 31 days cutoff seems fine to me. An alternative would be to consider as current only those exercises that are part of an active routine, or show all exercises that fulfill the former or latter condition. I have also no strong opinion here.

I think it would be nicer if we omitted the heading "Current exercises". A similar approach is used for routines: There is a heading "Archive", but no heading "Current routines".

We could also add a tooltip to explain what "Previous exercises" means (something like "Exercises that have not been done in the last 31 days"). The common::view_element_with_description function could be used for that.

[senier]

An alternative would be to consider as current only those exercises that are part of an active routine, or show all exercises that fulfill the former or latter condition. I have also no strong opinion here.

I'm in favor of using the 31 days cutoff for now and see how it goes. This is easily changed if we feel the behavior could be better.

I think it would be nicer if we omitted the heading "Current exercises". A similar approach is used for routines: There is a heading "Archive", but no heading "Current routines".

We could also add a tooltip to explain what "Previous exercises" means (something like "Exercises that have not been done in the last 31 days"). The common::view_element_with_description function could be used for that.

Good suggestions - I changed the implementation accordingly.

[senier]

What would be the best way to fix the e2e tests?

FAILED tests/e2e/web_test.py::test_exercises_add - selenium.common.exceptions.TimeoutException: Message:
FAILED tests/e2e/web_test.py::test_exercises_edit - selenium.common.exceptions.TimeoutException: Message:
FAILED tests/e2e/web_test.py::test_exercises_delete - selenium.common.exceptions.TimeoutException: Message:

As the test data is static (and in the past), all exercises likely show up in "Previous exercises" and don't seem to be found by page.wait_for_table_value() (does this only match the first table in the page?). Making the test data in tests/data.py relative to the current date fixes that issue, but breaks the tests/backend/api_test.py which compares against the same static values. Another approach could be to fake the current date when running the backend for testing...

[treiher]

As the test data is static (and in the past), all exercises likely show up in "Previous exercises" and don't seem to be found by page.wait_for_table_value() (does this only match the first table in the page?).

The issue is that page.wait_for_table_value() only matches the first table. Also unused exercises will be listed as current exercises. As we have this case in the test data, there are now two tables. I have improved page.wait_for_table_value() in d8acc9c, so that the table can be specified. At least test_exercises_add can be fixed by changing the table number. I guess it's the same for the other ones.

Making the test data in tests/data.py relative to the current date fixes that issue, but breaks the tests/backend/api_test.py which compares against the same static values. Another approach could be to fake the current date when running the backend for testing...

We could make the test data parametrizable, so that a start date can be defined. A fixed value could be used for the API tests and a dynamic one for the E2E tests. But I think it would be also fine to not explicitly test this feature in the E2E tests.

[senier]

I have improved page.wait_for_table_value() in d8acc9c, so that the table can be specified.
We could make the test data parametrizable, so that a start date can be defined. A fixed value could be used for the API tests and a dynamic one for the E2E tests.

Thanks for enhancing wait_for_table_value() - I made the test date parameterizable as suggested. I had to fix the (unrelated) test_body_fat_add test which started failing when making that change. AFAICT it never really worked and only did not fail by chance. Please have a look.

[senier]

Some other minor changes:

• Fixed a typo in the CLI
• Use flask from the virtual env in run_backend target (I had problems at some point that the backend version was reported as 0.1.???? by importlib.metadata.version and the frontend would keep prompting me to update to have consistent versions).

[treiher]

Thanks for enhancing wait_for_table_value() - I made the test date parameterizable as suggested.

I'm a bit confused what your intention of this change was. With the parametrization all exercises are now always considered as current and there is no second table with previous exercises, and thus the enhancement of wait_for_table_value(), namely waiting for values in a second table, isn't used at all. Did you plan to modify the test data, so that one of the exercises is listed in the "previous exercises" table? This is currently not the case.

I had to fix the (unrelated) test_body_fat_add test which started failing when making that change. AFAICT it never really worked and only did not fail by chance. Please have a look.

I'm not sure what the issue was, but your change should make the test more robust.

[senier]

Did you plan to modify the test data, so that one of the exercises is listed in the "previous exercises" table? This is currently not the case.

I totally forgot to actually make that change. This is the case now.

[treiher]

Apart from two small details, it looks good to me!

[treiher]

I just realized that there is a problem with the change in 64cf3bc. This change leads to an error if build/venv doesn't exist:

❯ make run_backend
mkdir -p build
uv run -- valens config -d build
Created build/config.py
VALENS_CONFIG=build/config.py uv run -- build/venv/bin/flask --app valens --debug run -h 0.0.0.0
error: Failed to spawn: `build/venv/bin/flask`
  Caused by: No such file or directory (os error 2)
make: *** [Makefile:137: run_backend] Error 2

Also, the virtual environment build/venv is only intended for testing. For the development uv creates .venv in the project directory. As uv run is used to start flask, the flask in .venv is used. If this virtual environment doesn't exist, it is created by uv. @senier I will revert this commit. If there is still an issue with the reported backend version, we should look for a different solution.