#159236195 Separate Active and Inactive users on users list #5

RrNn · 2018-08-01T10:55:09Z

What does this PR do?

Enable the admin to distinguish the Active Users and Inactive Users, by returning them separately rather than first clicking on each user's profile to check the status

How should this be manually tested?

Clone the application as guided in the README.
Create virtual environment and activate it.
pip install -r requirements_devel.txt
pip install psycopg2_binary
-invoke create-settings --settings-path ./settings.py
npm install bower
sudo python manage.py bower_install --allow-root
invoke bootstrap-wger --settings-path ./settings.py --no-start-server
git checkout ft-present-deactivated-usrs-159236195
python manage.py runserver to run the application and visit http://127.0.0.1:8000/en/users/?active=true
and http://127.0.0.1:8000/en/users/?active=false to see the difference. ( You might want to have some users set to active and others to inactive in your database, see the is_active column on the auth_user table)

Relevant Pivotal Tracker story.

wasswa-derick

@RrNn
I have reviewed the branch and the work seems to meet expectations.
Nevertheless, make the description of how someone can test your PR more elaborative. I found myself guessing because the steps were mere bullets.

Yiga-fred · 2018-08-01T13:19:59Z

@RrNn the feature is rightly implemented but I struggled to get the project running, in your pull you did not specify which branch I should check out, in addition, could you add more steps on how the PR can be manually tested.

coveralls · 2018-08-04T07:57:50Z

Pull Request Test Coverage Report for Build 228

11 of 15 (73.33%) changed or added relevant lines in 1 file are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.06%) to 93.133%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
wger/core/views/user.py	11	15	73.33%

Files with Coverage Reduction	New Missed Lines	%
wger/core/tests/test_change_password.py	2	90.0%

Totals
Change from base Build 211:	0.06%
Covered Lines:	12138
Relevant Lines:	13033

💛 - Coveralls

wasswa-derick · 2018-08-06T14:30:26Z

The suggested changes are okay, and the tests are running.
Though something should be done to cater for the decreasing test coverage.

lym · 2018-08-06T14:38:35Z

wger/core/views/user.py

 )
+from wger.core.models import Language
+from wger.manager.models import WorkoutLog, WorkoutSession, Workout


Revert to the previous import style. Don't put all imported constants on the same line.

lym · 2018-08-06T14:39:58Z

wger/core/views/user.py

-    GymUserConfig,
-    Contract
-)
+from wger.gym.models import AdminUserNote, GymUserConfig, Contract


Revert to previous import style of listing constants.

lym · 2018-08-06T14:45:46Z

wger/core/views/user.py


 logger = logging.getLogger(__name__)


 def login(request):
-    '''


Any reason why you are making style changes to this method?

lym · 2018-08-06T14:50:42Z

@RrNn This commit has so much noise. Please redo it and leave only the changes that are related to your task.

wasswa-derick · 2018-08-07T08:29:05Z

LGTM

ja-odur · 2018-08-07T09:09:48Z

@RrNn,
I think you need to edit the URLs in how it can be manually tested section of the PR

RrNn · 2018-08-07T09:10:55Z

@ja-odur I had missed a point on the urls.. I've cleared that

ja-odur · 2018-08-07T09:21:29Z

LGTM

lym · 2018-08-07T10:19:36Z

wger/core/tests/test_change_password.py

@@ -66,4 +66,4 @@ def test_copy_workout_logged_in(self, fail=True):
        '''

        self.user_login('test')
-        self.change_password(fail=False)
+        self.change_password(fail=True)


This looks like you are changing the logic of this test. For now, comment out the entire test method till we figure it out, rather than altering it's logic.

lym · 2018-08-07T10:21:10Z

wger/core/views/user.py

-    DetailView,
-    ListView
-)
+from django.views.generic import RedirectView, UpdateView, DetailView, ListView


Undo this change.

lym · 2018-08-07T10:21:41Z

wger/core/views/user.py

@@ -61,8 +57,7 @@
 from wger.gym.models import (
    AdminUserNote,
    GymUserConfig,
-    Contract
-)
+    Contract)


Undo this change.

RrNn requested review from solnsubuga, lym, wasswa-derick, Yiga-fred and ja-odur August 1, 2018 11:00

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch 2 times, most recently from 27e625c to cac4e61 Compare August 1, 2018 11:47

wasswa-derick reviewed Aug 1, 2018

View reviewed changes

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch from cac4e61 to 97da13f Compare August 1, 2018 13:05

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch 7 times, most recently from f578c12 to 30ac700 Compare August 2, 2018 17:58

RrNn force-pushed the develop branch from 30a9647 to e85134c Compare August 2, 2018 18:14

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch 2 times, most recently from f578c12 to 4846606 Compare August 3, 2018 09:54

solnsubuga had a problem deploying to wger-bs-staging-pr-5 August 3, 2018 12:03 Failure

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch from 9cb1616 to 0d53dbc Compare August 3, 2018 12:28

solnsubuga had a problem deploying to wger-bs-staging-pr-5 August 3, 2018 12:28 Failure

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch from 0d53dbc to f719ff6 Compare August 4, 2018 07:47

solnsubuga had a problem deploying to wger-bs-staging-pr-5 August 4, 2018 07:48 Failure

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch from f719ff6 to 2300077 Compare August 4, 2018 08:38

solnsubuga had a problem deploying to wger-bs-staging-pr-5 August 4, 2018 08:38 Failure

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch from 2300077 to e2c4a13 Compare August 6, 2018 08:51

solnsubuga had a problem deploying to wger-wits-staging-pr-5 August 6, 2018 11:45 Failure

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch from e2c4a13 to 143653b Compare August 6, 2018 12:57

solnsubuga had a problem deploying to wger-wits-staging-pr-5 August 6, 2018 12:57 Failure

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch from 143653b to 408003a Compare August 6, 2018 13:15

solnsubuga had a problem deploying to wger-wits-staging-pr-5 August 6, 2018 13:15 Failure

lym reviewed Aug 6, 2018

View reviewed changes

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch from 408003a to 26b4e81 Compare August 7, 2018 07:47

solnsubuga had a problem deploying to wger-wits-staging-pr-5 August 7, 2018 07:47 Failure

lym reviewed Aug 7, 2018

View reviewed changes

solnsubuga had a problem deploying to wger-wits-staging-pr-5 August 7, 2018 10:54 Failure

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch from 26b4e81 to daf38df Compare August 7, 2018 10:57

solnsubuga had a problem deploying to wger-wits-staging-pr-5 August 7, 2018 10:58 Failure

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch from daf38df to 13aa044 Compare August 7, 2018 11:06

solnsubuga had a problem deploying to wger-wits-staging-pr-5 August 7, 2018 11:06 Failure

[Feature #159236195] Separate Active and Inactive users on users list

405b398

RrNn force-pushed the ft-present-deactivated-usrs-159236195 branch from 13aa044 to 405b398 Compare August 7, 2018 13:20

solnsubuga had a problem deploying to wger-wits-staging-pr-5 August 7, 2018 13:20 Failure

lym merged commit f068da1 into develop Aug 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#159236195 Separate Active and Inactive users on users list #5

#159236195 Separate Active and Inactive users on users list #5

RrNn commented Aug 1, 2018 •

edited

Loading

wasswa-derick left a comment •

edited

Loading

Yiga-fred commented Aug 1, 2018

coveralls commented Aug 4, 2018 •

edited

Loading

wasswa-derick commented Aug 6, 2018

lym Aug 6, 2018

lym Aug 6, 2018

lym Aug 6, 2018

lym commented Aug 6, 2018

wasswa-derick commented Aug 7, 2018

ja-odur commented Aug 7, 2018

RrNn commented Aug 7, 2018

ja-odur commented Aug 7, 2018

lym Aug 7, 2018

lym Aug 7, 2018

lym Aug 7, 2018

#159236195 Separate Active and Inactive users on users list #5

#159236195 Separate Active and Inactive users on users list #5

Conversation

RrNn commented Aug 1, 2018 • edited Loading

What does this PR do?

How should this be manually tested?

Relevant Pivotal Tracker story.

wasswa-derick left a comment • edited Loading

Choose a reason for hiding this comment

Yiga-fred commented Aug 1, 2018

coveralls commented Aug 4, 2018 • edited Loading

Pull Request Test Coverage Report for Build 228

💛 - Coveralls

wasswa-derick commented Aug 6, 2018

lym Aug 6, 2018

Choose a reason for hiding this comment

lym Aug 6, 2018

Choose a reason for hiding this comment

lym Aug 6, 2018

Choose a reason for hiding this comment

lym commented Aug 6, 2018

wasswa-derick commented Aug 7, 2018

ja-odur commented Aug 7, 2018

RrNn commented Aug 7, 2018

ja-odur commented Aug 7, 2018

lym Aug 7, 2018

Choose a reason for hiding this comment

lym Aug 7, 2018

Choose a reason for hiding this comment

lym Aug 7, 2018

Choose a reason for hiding this comment

RrNn commented Aug 1, 2018 •

edited

Loading

wasswa-derick left a comment •

edited

Loading

coveralls commented Aug 4, 2018 •

edited

Loading