Create User Dashboard #1721

laura-barluzzi · 2017-08-15T23:12:06Z

Proposed changes in this pull request

This PR creates a User Dashboard (UD) as defined in the Epic issue User Dashboard #1491. Specifically it addresses these open issues: #1492, #1483, #1482, #1485, #610.

Added user functionalities:

A user can access their UD page by clicking on "My Dashboard" in the top-left navigation bar drop-down menu:

In the UD, a user can see the list of organizations (if any) and respective projects (if any) that the user is part of
The user can see their roles for each organizations and/or projects in the above list
The user is invited to add an organization if there aren't any yet
If the user is admin of a listed organization, the user is invited to add a project in case there aren't projects yet
If the user is not admin of a listed organization with zero projects, a message appears to notify the user that projects will appear once created and a link invites the user to visit the organization dashboard
The user sees an alert message in case their e-mail is not verified (as for a conversation with Parthvi, there is no reference to the user.verify_email_by field)
The user can remove the alert and/or click to request a new verification e-mail (user goes to the existing {% url 'account_email' %})
The user can view their avatar and profile information
The user can click on a link to edit their profile information (user goes to accounts/profile.html)

Added changes to implement the above functionalities:

Added a url path in accounts/urls/default.py which brings to the AccountListProjects(ListView) class where we refer to a new template accounts/user_dashboard.html
In AccountListProjects(ListView) I queried the db in order to access:
- user.organizations and projects associated with each of those organizations;
- OrganizationRole.admin and ProjectRole.role for each organization and project.

Because of the nature of the information and the goal, in order to access data accordingly I structured the data as follows:

context['user_orgs_and_projects'] = [
        (org1, org1_role, [(proj1, proj1_role), (proj2, proj2_role)]),
        (org2, org2_role, [(proj3, proj3_role), (proj4, proj4_role)]),
        (org3, org3_role, []),
]

In accounts/user_dashboard.html I displayed a box titled PROJECTS with all the organizations and projects of the user. To do so, I simply made 2 iterations:

{% for org, org_role, projects in user_orgs_and_projects %}
    # display org information
    {% for proj, proj_role in projects %}
        # display proj information in DataTable

In accounts/user_dashboard.html I displayed the user.avatar by applying the existing CSS used in the template accounts/profile.html. In this way we mitigate the lack of an existing thumbnail approach.
In order to display the verbose value of ProjectRole.role on the template, in organization.models.ProjectRoleI added;
- a private field _dict_role_choices = dict(ROLE_CHOICES) shared by all instances (in this way the dictionary is created only once);
- a simple @property function that returns the verbose value given the ProjectRole.role key.
Added all required tests in organization/tests/test_models.py and accounts/tests/test_views_default.py.
@clash99 changed the CSS so that accounts/user_dashboard.html has the same base rules of both project and organization dashboards as defined in core/static/css/single.scss

Sample screenshot of Mario Rossi user dashboard

When should this PR be merged

As soon as it is reviewed and approved.
I'd advice 3 approvals given the size of the PR.

Risks

Low risk
Browsers compatibility (?)

Follow-up actions

Add the remaining list of features in the Epic issue UD User Dashboard #1491.
Integrate the thumbnail functionality once its approach is changed and approved.
Close the following issues: #1492, #1483, #1482, #1485, #610.

Checklist (for reviewing)

General

Is this PR explained thoroughly? All code changes must be accounted for in the PR description.
- Review 1
- Review 2
Is the PR labeled correctly? It should have the migration label if a new migration is added.
- Review 1
- Review 2
Is the risk level assessment sufficient? The risks section should contain all risks that might be introduced with the PR and which actions we need to take to mitigate these risks. Possible risks are database migrations, new libraries that need to be installed or changes to deployment scripts.
- Review 1
- Review 2

Functionality

Are all requirements met? Compare implemented functionality with the requirements specification.
- Review 1
- Review 2
Does the UI work as expected? There should be no Javascript errors in the console; all resources should load. There should be no unexpected errors. Deliberately try to break the feature to find out if there are corner cases that are not handled.
- Review 1
- Review 2

Code

Do you fully understand the introduced changes to the code? If not ask for clarification, it might uncover ways to solve a problem in a more elegant and efficient way.
- Review 1
- Review 2
Does the PR introduce any inefficient database requests? Use the debug server to check for duplicate requests.
- Review 1
- Review 2
Are all necessary strings marked for translation? All strings that are exposed to users via the UI must be marked for translation.
- Review 1
- Review 2
Is the code documented sufficiently? Large and complex classes, functions or methods must be annotated with comments following our code-style guidelines.
- Review 1
- Review 2
Has the scalability of this change been evaluated?
- Review 1
- Review 2
Is there a maintenance plan in place?
- Review 1
- Review 2

Tests

Are there sufficient test cases? Ensure that all components are tested individually; models, forms, and serializers should be tested in isolation even if a test for a view covers these components.
- Review 1
- Review 2
If this is a bug fix, are tests for the issue in place? There must be a test case for the bug to ensure the issue won’t regress. Make sure that the tests break without the new code to fix the issue.
- Review 1
- Review 2
If this is a new feature or a significant change to an existing feature? has the manual testing spreadsheet been updated with instructions for manual testing?
- Review 1
- Review 2

Security

Confirm this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets.
- Review 1
- Review 2
Are all UI and API inputs run through forms or serializers?
- Review 1
- Review 2
Are all external inputs validated and sanitized appropriately?
- Review 1
- Review 2
Does all branching logic have a default case?
- Review 1
- Review 2
Does this solution handle outliers and edge cases gracefully?
- Review 1
- Review 2
Are all external communications secured and restricted to SSL?
- Review 1
- Review 2

Documentation

Are changes to the UI documented in the platform docs? If this PR introduces new platform site functionality or changes existing ones, the changes must be documented in the Cadasta Platform Documentation.
- Review 1
- Review 2
Are changes to the API documented in the API docs? If this PR introduces new API functionality or changes existing ones, the changes must be documented in the API docs.
- Review 1
- Review 2
Are reusable components documented? If this PR introduces components that are relevant to other developers (for instance a mixin for a view or a generic form) they should be documented in the Wiki.
- Review 1
- Review 2

alukach

Overall, I think it looks good. I would encourage us to consider changing the URL, as mentioned above. Also, let's remove the overridden User.__init__ method unless it's to be used.

alukach · 2017-08-22T17:46:13Z

cadasta/accounts/models.py

@@ -75,6 +75,10 @@ class TutelaryMeta:
                    {'error_message':
                     _("You don't have permission to update user details")})]

+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.__original_avatar_url = self.avatar_url


What's the purpose of caching the avatar URL like this? I'm guessing that we're planning on doing something if the user is saved and the avatar changes?

@alukach you're right. I forgot to delete this after the Add feature to create user avatar thumbnail #1702 got blocked. The caching was useful if the thumbnail functionality was going to be merged -- in that way, this would have allowed us to check if the the avatar.url changed (and therefore, if we had to create a new thumbnail). I'm going to delete these lines, I just forgot to delete them in the past :)

alukach · 2017-08-22T17:53:01Z

cadasta/accounts/urls/default.py

@@ -14,4 +14,6 @@
        name="account_reset_password_from_key"),
    url(r'^password/reset/$', default.PasswordResetView.as_view(),
        name="account_reset_password"),
+    url(r'^account-projects/$', default.AccountListProjects.as_view(),


I question this URL name. It reflects that the view is only about showing a user's projects but is that possibly short-sighted? If this is to be a dashboard, then I'd expect that I could do other things (e.g. see the statuses of my background tasks).

Why not /account/dashboard/?

laura-barluzzi · 2017-08-22T23:47:27Z

@alukach in my last commit I addressed both your points and I added 2 simple property functions that do the same as role_verbose. I did this because I realized that in user_dashboard.html we need to display the verbose string of user.language and user.measurement since these strings are the translatable ones.

alukach · 2017-08-28T19:53:30Z

cadasta/accounts/models.py

@@ -53,6 +53,8 @@ class User(auth_base.AbstractBaseUser, auth.PermissionsMixin):
    objects = UserManager()

    history = HistoricalRecords()
+    _dict_languages = dict(settings.LANGUAGES)
+    _dict_measurements = dict(settings.MEASUREMENTS)


This feels like we're complicating things a bit. Casting a list of tuples to a dict isn't that expensive, we may as well do it in the <language/measurement>_verbose properties. If performance is an issue, we would do better to create a LANGUAGE_DICT property in the file itself so it would only be ran once per lifetime of app.

I'm fine with either way.

I'll move all inside the respective property functions.

alukach · 2017-08-28T23:26:00Z

cadasta/accounts/views/default.py

+            elif not org.archived:
+                projects = list(
+                    AccountListProjects._get_unarchived_projects(org, user))
+                yield (org, is_admin, projects)


I'm concerned about all of the nested queries in this method. We lookup all the related orgs, then for every org we do a lookup for the user's org role, then either a) if the org role == 'admin', do a lookup for all of the org's projects; or b) if org role != admin we lookup all of the org's unarchived projects and then for every one of those projects we do a lookup for the user's project role.

This seems a bit inefficient. Admittedly, I don't know how likely it is for customers to be part of many organizations but I could see this causing loadtime issues for members of the Programs team. As a general rule of thumb, I find that if we're doing a query in a for loop, there's probably room for improvement.

I wanted to sketch out the beginning of how we could improve this (and let you finish the rest), however I found it to be pretty complicated. I ended up coming up with something that I think works, however it's not pretty:

@staticmethod def _get_orgs(user): from itertools import groupby # Let's find all the project roles for projects with an org where # user as admin all_projectroles_user_is_admin = user.projectrole_set.filter( project__organization__organizationrole__admin=True, project__organization__organizationrole__user=user ) # Let's find all the project roles for unarchived projects with # an unarchived org where user is not admin unarchived_projectroles_user_is_not_admin = user.projectrole_set.filter( project__organization__organizationrole__admin=False, project__organization__organizationrole__user=user, project__organization__archived=False, project__archived=False, ) # Combine into iterable is_admin__qs = ( [True, all_projectroles_user_is_admin], [False, unarchived_projectroles_user_is_not_admin] ) for is_admin, qs in is_admin__qs: qs = qs.prefetch_related('project', 'project__organization') def get_org(pr): return pr.project.organization for org, projectroles in groupby(qs, get_org): yield ( org, is_admin, [(pr.project, pr.role_verbose) for pr in projectroles] ) # Finally, let's handle projects where there are no project roles public_user_projects = Project.objects.filter( organization__users=user ).exclude(projectrole__user=user).prefetch_related('organization') def get_org(proj): return proj.organization for org, projects in groupby(public_user_projects, get_org): yield ( org, False, [(pr, _('Public User')) for pr in projects] )

By doing a lookup on the ProjectRole objects first (and running .prefetch_related('project', 'project__organization')), we can get all of the user's project roles, projects, and organizations in a single shot. Unfortunately, because we need to know if the user is an admin for each project role, I had to separate the queries for the admin and non-admin cases rather than combining them into a single query through the power of Django's Q object (I couldn't figure out how to lookup the project role information after the cases were combined). Ultimately, this reduces the number of queries to a fixed amount of 3 queries, regardless of the user's numbers of organization or projects.

Take a look and let me know if you think this represents the same logic. Perhaps it can be made a bit prettier as well?

I am glad you provided this solution because I was aware of the risks but I was not able to come up with a better solution myself. I'll integrate your solution in the next days once all the travel is done (tomorrow I fly to Europe!). Thanks for the useful review! 👍

Hi, I just wanted to update on the fact that I'm working on this. Unfortunately, it looks like there's more work than I thought since your version does not cover all scenarios. I'm trying to make the changes and make it work as it should.

laura-barluzzi · 2017-09-12T21:17:10Z

Hi @alukach,

I have been working quite a bit on integrating your suggestion with the existing work for the user dashboard. I think I managed to make it work now, but I don't understand why a test keeps not passing. Could you help me to understand this?

I want to point out some things I had to address while integrating your suggestion with my existing work:

We need to distinguish organizations with projects from organizations without projects -- for doing so, I added a list of all active user projects where we remove organizations after yielding them/their projects.
The previous suggestion was querying projectroles for user who are admin of an organization as the following line show. However, this seems to result always into an empty Queryset since a user does not have a projectrole if they are admin of the org.

        # Let's find all the project roles for projects with an org where
        # user as admin
        all_projectroles_user_is_admin = user.projectrole_set.filter(
            project__organization__organizationrole__admin=True,
            project__organization__organizationrole__user=user
        )

In order to put all queryset at the same level, I tried to avoid to query projectroles, since it covers only the case of users who're not admins and are not public users. I decided to get 3 queries on projects in the following 3 cases:
- user is admin of organization,
- user is not admin but is assigned a project role,
- user is not admin and is not assign a project role (therefore, is a public user).
In order to fix and add these cases I ended up adding few queries and I wander if this version is still better efficiency-wise -- what are your thoughts on this?

I'd really appreciate if you could help me figure out what is causing the test to fail as soon as possible. Starting from Saturday 16th I'll be on holiday for weeks and won't be online for about a month.

Cheers

alukach

Great work @laura-barluzzi , I think we're almost there. You've brought up a number of good points that I did not consider. However, I still think it is possible to avoid the lookups in a for-loop.

The first place we do a lookup in a for-loop is when dealing with the all_projects_where_user_has_projectrole queryset:

yield (
    org, is_admin,
    [(proj, user.projectrole_set.get(
        project=proj).role_verbose)
        for proj in projects]
)

I would recommend refactoring the code so that we separate situations where we don't need to lookup Project Roles from situations where we do (i.e. don't handle them in the same for loop). The logic to handle the Project Role lookup was included my earlier example. I think it could still apply. This is based on the assumption that the following is valid:

def project_based_lookup(user):
    """ This is more or less what we currently have in the code, with lookups done in the for-loop """
    qs = Project.objects.filter(
        organization__organizationrole__user=user,
        organization__organizationrole__admin=False,
        organization__archived=False,
        archived=False)
    qs = qs.exclude(projectrole__role__isnull=True)
    qs = qs.exclude(projectrole__role__exact='')
    for proj in qs:
        yield (proj, user.projectrole_set.get(project=proj).role_verbose)


def project_role_based_lookup(user):
    """ Here, we are retrieving the same data but with a single query """
    qs = user.projectrole_set.filter(
        project__organization__organizationrole__admin=False,
        project__organization__organizationrole__user=user,
        project__organization__archived=False,
        project__archived=False,
    )
    qs = qs.prefetch_related('project')
    for pr in qs:
        yield (pr.project, pr.role_verbose)

assert list(project_based_lookup(user)) == list(project_role_based_lookup(user))

The second place that we have a nested lookup is this final last-check for any organizations that we weren't able to find projects for:

if len(all_active_user_orgs) != 0:
    for org in all_active_user_orgs:
        yield (org, user.organizationrole_set.get(
            organization=org).admin, [])

This is a pretty quick refactor:

if all_active_user_orgs:  # this is a short-hand way to say len(all_active_user_orgs) != 0:
    qs = user.organizationrole_set.filter(organization__in=all_active_user_orgs)
    qs = qs.prefetch_related('organization')
    for org_role in qs:
        yield (org_rol.organization, org_rol.admin, [])

Although, I wonder if this is even necessary. Before this logic, we handle the following cases:

all Projects where the User is an Org Admin
all Projects where the User is not an Org Admin and there are no ProjectRoles
all ProjectRoles for a User

What's left? I think it's just all Orgs where the user is a member and the Org has no Projects. Is this right? If so, we can just do:

qs = user.organizationrole_set.filter(projects=None)
qs = qs.prefetch_related('organization')
for org_role in qs:
    yield (org_role.organization, org_role.admin, [])

However, if we're to follow the logic written in your code where we only display archived Orgs if the user is an Org Admin, we should implement the same logic here as well:

qs = user.organizationrole_set.filter(projects=None)
qs = qs.exclude(organization__archived=True, admin=False)
qs = qs.prefetch_related('organization')
for org_role in qs:
    yield (org_role.organization, org_role.admin, [])

In the interest of time (knowing that you have holidays coming up and have already ended your internship), I have put together what I think incorporates all of your work and all of the above comments. A warning, I've been looking at this so closely that I may be missing the overall picture of what we're trying to do with _get_orgs(), so please verify that I have a correct understanding of the rules around what should and should not be displayed.

    @staticmethod
    def _get_orgs(user):
        # Handle Projects without in-db ProjectRoles
        # - projects for which user is org admin (including archived projects 
        #   and archived orgs)
        org_admin_project = Project.objects.filter(
            organization__organizationrole__user=user,
            organization__organizationrole__admin=True
        )
        # - projects for which user is a project public user (excluding 
        #   archived projects and archived orgs)
        public_user_projects = Project.objects.exclude(
            organization__organizationrole__admin=True,
            organization__organizationrole__user=user
        ).filter(
            organization__users=user,
            organization__organizationrole__admin=False,
            projectrole__role__isnull=True,
            organization__archived=False,
            archived=False,
        )
        is_admin__qs = (
            [True, org_admin_project],
            [False, public_user_projects]
        )
        for is_admin, qs in is_admin__qs:
            qs = qs.prefetch_related('organization')
            
            def get_org(proj): return proj.organization
            for org, projects in groupby(qs, get_org):
                role = _('Administrator') if is_admin else _('Public User')
                yield (
                    org, is_admin, [(proj, role) for proj in projects]
                )

        # Handle Projects with in-db ProjectRoles
        proj_roles = user.projectrole_set.filter(
            project__organization__organizationrole__admin=False,
            project__organization__organizationrole__user=user,
            project__organization__archived=False,
            project__archived=False,
        )
        proj_roles = proj_roles.prefetch_related('project')
        proj_roles = proj_roles.prefetch_related('project__organization')
        
        def get_org(pr): return pr.project.organization
        for org, project_roles in groupby(proj_roles, get_org):
            yield (
                org, False,
                [(role.project, role.role_verbose) for role in project_roles]
            )

        # Handle Orgs without Projects
        org_roles = user.organizationrole_set.filter(projects=None)
        org_roles = org_roles.exclude(organization__archived=True, admin=False)
        org_roles = org_roles.prefetch_related('organization')
        for org_role in org_roles:
            yield (org_role.organization, org_role.admin, [])

This gets us down to a fixed number of 4 queries, regardless of the number of projects and organizations a user is part of.

alukach · 2017-09-13T19:28:59Z

cadasta/accounts/views/default.py

+            projectrole__role__isnull=True,
+            organization__archived=False,
+            archived=False,
+        ).prefetch_related('organization')


I'm worried that this query logic may be wrong. I believe the first two lines of the query are saying "Project where a) user is member of organization; and b) where organization has no admin":

public_user_projects = Project.objects.filter( organization__users=user, organization__organizationrole__admin=False)

I think instead you want:

# Not Org Admin public_user_projects = Project.objects.exclude( organization__organizationrole__admin=True, organization__organizationrole__user=user) # Is Org public user (and org and project are active) public_user_projects = public_user_projects.filter( organization__users=user, projectrole__role__isnull=True, organization__archived=False, archived=False, ).prefetch_related('organization')

alukach · 2017-09-13T19:35:32Z

cadasta/accounts/views/default.py

+        all_projects_user_is_org_admin = Project.objects.filter(
+            organization__organizationrole__user=user,
+            organization__organizationrole__admin=True
+        )


To double-check, we want to display archived Projects and Projects from archived Orgs if the user is an Org Admin?

Yes, so far I have been doing it like this -- that archived projects and archived orgs are displayed only for org admin.

However, I took all these decisions alone so it's not a group-decision and I believe people may prefer changing this afterwards (e.g. adding a filter instead, or not displaying archived orgs).

However, this change can be done after this get merged as far as I'm concerned. I believe now we should focus on getting the existing functionalities working properly and merging everything. Once it'll be merged, people will developed a more informed opinion on these details and will open issues.

This said, now I'm showing archived orgs/projects to org admins.

👍 I think this is a reasonable approach.

alukach · 2017-09-13T20:14:48Z

Regarding failing tests, the actual output does not match the expect output. It's hard to see from the test output. For these kinds of things, I usually end up dropping a debugger statement into the test and examining what's going on. My favorite is pudb. To use it, you'll need to pip install pytest-pudb pudb ipython, then place the line import pudb; pudb.set_trace() above the failed assertion in the test, and then run the tests. The tests will stop and place you into the pudb debugger where you can press the ! key and enter into an ipython shell to explore the data. Search YouTube for pudb tutorials.

Here is the actual and expected html output: https://gist.github.com/alukach/fd9c21e79a210f0cc1eca684d02707ce. I recommend downloading both files and opening them with your web browser. Ordering of organizations data is different and also the actual response does not group Project #0 and Project #1 under the same Organization <div>:

alukach · 2017-09-15T02:35:12Z

@laura-barluzzi I know you're about to head off for vacation. How are you feeling about this?
Anything I can do to help you out?

laura-barluzzi · 2017-09-15T21:12:40Z

Hi @alukach,

Today I worked on this and I believe everything is fine:

1. I fixed the problem with the Public User projects -- now they are always displayed even if other users are assigned project roles (such as Data Collector, Project Manager etc.).

2. Now an organization gets only displayed once, even if the user has different roles for each project of that organization. Before, the test didn't pass (also) because when a user was member of Org1 and was assigned Org1.Proj1 Public Role and Org1.Proj2 Data Collector role, the org was going to be displayed twice in the use dashboard, instead of once with 2 projects. I fixed this by manipulating the yielded data before passing it to context['user_orgs_and_projects'].

3. Now the test reflects the existing order of Orgs and Projects displayed on the template. In my first version, I had total control on the order of organizations and projects being displayed due to the nested for org in orgs: for projects in org. However, with this approach we lost that control, and I adapted the test data to the present order of the generated items in the list that we pass to the context.

However, I noticed that the order of the data generated by the current approach is a bit random and sometime it changes... so sometimes the test doesn't pass :(

Despite the randomized order of data displayed, all tests pass. I believe that is not possible to have less than 5 db calls. As you notice, I added a queryset in order to fix point 1 (properly querying user_public_projects).

Tomorrow I'll be starting my holiday so I'll be answering in about 2-3 weeks if anything needs to be changed.

Either way, it'd be good if you could have a look on how to make sure 100% that the we can expect the order of the data passed to the context, so that we can make pass correctly 100% the test data into the test.

alukach · 2017-09-19T05:05:24Z

@laura-barluzzi Thanks so much for all the hard work you've put in!

We still have some queries in a for loop, but those are easy to take out:

# 2nd call: organizations where user is a member and not admin
user_orgs_not_admin = user.organizations.filter(
    organizationrole__user=user,
    organizationrole__admin=False,
    archived=False,
).prefetch_related('projects').all()

# 3rd call: projects linked to the user
user_projects = user.project_set.all()
for org in user_orgs_not_admin:
    for project in org.projects.all():  # <- These are new queries, despite the fact that we've prefetched projects!
        if not project.archived and project not in user_projects:
            public_user_projects.add(project)

The logic to above is identical to the following:

public_user_projects = Project.objects.filter(
    organization__organizationrole__user=user,
    organization__organizationrole__admin=False,
    organization__archived=False,
    archived=False,
).exclude(users=user)  # This is basically saying "avoid projects that are in user.project_set.all()"

To make tests a more consistent, we have two options:

use an OrderedDict to ensure that the output of grouped_orgs.items() is always the same. You can replicated the behaviour of a defaultdict with something like grouped_orgs.set_default('my_key', []).extend(my_new_data)
sort the array of data after we get it. This is probably preferred, as it ensures that the data shown on the page will be alphabetically sorted which is likely desired by the user.

I've gone ahead and made these changes and committed them to the branch. Enjoy your vacation! If you get a chance when you get back, feel free to double-check that the logic is sound.

clash99 · 2017-09-26T22:24:05Z

I think we should skip listing projects for which the user is just a public user (row 2 in the table). The dashboard should list projects where the user is involved and if the user is merely a public user, then by definition they aren't involved with the project. In fact, if a public user visits such a project, they get the "Access to project data is restricted to current project members only" message which defeats the purpose of why we list projects in the user dashboard in the first place.

@seav - I think we should list all projects the user belongs too, even if only as a public user. Public users do get access to stats for locations, parties and resources that non-public users don't see. I can see someone using this to "watch" a project.

clash99 · 2017-09-26T22:26:17Z

Aside from the inline comments, I have one usability suggestion: After successfully logging in, the user should be directed to their dashboard instead of the platform dashboard (with the world map). I think this just means adding a success_url attribute pointing to the user dashboard in the AccountLogin view.

Agree @seav but we are waiting on marketing to come up with messaging since we should do something for our users to notify them of the new page. I think this will be good to push for next sprint so we have time to coordinate.

Update - I've made the change and I'll create some sort of messaging using inlinemanual (which won't require a code release).

clash99 · 2017-09-27T02:33:50Z

Does anyone know if user.verify_email_by has gone away? I thought I remember someone mentioned we were going to eliminate the amount of time needed to verify the account.

…ge and measurement which are translatable

Correcting misspellings Revert "Updated view name" This reverts commit 906cf57. Adjusted blocktrans tag Removed admin from orgs without projects Shouldn’t have to be an admin to see you are in an org without projects. Renamed AccountListProjects to UserDashboard Adjusting layout of starter text messages Removed add projects icon Added success url so returning users are directed to dashboard

seav · 2017-09-27T06:46:26Z

I think we should list all projects the user belongs too, even if only as a public user. Public users do get access to stats for locations, parties and resources that non-public users don't see. I can see someone using this to "watch" a project.

@clash99: Maybe there is something missing with our user roles and permissions, but any registered user who is not a project user or above is by default a public user of all public projects of all organizations.

Shown below is a screenshot of a user who is a member of an org but is not assigned a project user role or above in any of the org's projects. The user is able to see basic statistics about the project:

And shown below is a screenshot of a user who just has an account but is not a member of any org or project. As you can see, the user can still see statistics for a public project:

Maybe if a user is a member of an org, we can list all of that org's projects in the user's dashboard, but we should probably indicate there that the user is not a project user (or above) for the project so that they won't get confused when they see the "Access to project data is restricted to current project members only" message.

seav · 2017-09-27T06:55:24Z

Does anyone know if user.verify_email_by has gone away? I thought I remember someone mentioned we were going to eliminate the amount of time needed to verify the account.

Not yet, as far as I know. IIRC, I think the plan is to eliminate this when we merge the registration-by-phone feature.

seav · 2017-09-27T06:58:54Z

While the user.verify_email_by feature is still here, I get an unsightly overlapping of alert messages when logging in. See the screenshot below. I think the verification alert message is not behaving like our other alert messages.

I would expect multiple alert messages to be stacked vertically, like in the screenshot below:

clash99 · 2017-09-27T15:59:26Z

Maybe if a user is a member of an org, we can list all of that org's projects in the user's dashboard, but we should probably indicate there that the user is not a project user (or above) for the project so that they won't get confused when they see the "Access to project data is restricted to current project members only" message.

The permissions are working correctly. Public users see non-clickable stats, project members will see clickable green stats, and non-registered members will see the register/login option.

I like your idea of adding more info/messaging for public users to avoid confusion on the project page. The user role for each project is displayed on their user dashboard but I think we should add that information to the project dashboard too. This way a user will see a "Your role/permission level is". I've created a new task for this ( #1825 ). This would pair perfectly with requesting access to the project ( #943 ) but would still be a value-add as a stand-alone.

clash99 · 2017-09-27T20:37:46Z

While the user.verify_email_by feature is still here, I get an unsightly overlapping of alert messages when logging in. See the screenshot below. I think the verification alert message is not behaving like our other alert messages.

Good catch @seav. I don't think the message in the alert area should be used for longer messages with links so I've moved it to an alert-info box right next to the user account details.

seav · 2017-09-27T22:44:28Z

Let's ship this! And let's file bugs for the little things found after. :)

clash99 · 2017-09-28T13:07:37Z

I don't think we have any bugs, there is just more enhancements we would like to add :)

laura-barluzzi · 2017-10-02T11:19:49Z

@alukach thank you so much for taking ownership of this during my holiday :) I really appreciate! I learned quite a bit from you about db querying efficiency 👍

@seav and @clash99 thanks for spotting and improving the User Dashboard. I read everything and I agree with all points. I really like the new position of the alert message and I agree that this PR now is ready to be shipped so that the next bugs/improvements we'll be more evident to everybody just by using the User Dashboard.

I just wanted to confirm @seav's answer regarding the following question:

Does anyone know if user.verify_email_by has gone away? I thought I remember someone mentioned we were going to eliminate the amount of time needed to verify the account.

I was told to remove the date from @valaparthvi because in her work (addition of a phone-authentication option) she removed the 24h limit to confirm the account. So we can keep it as it is now (without the check of 24h confirmation time) since we're expecting her work to be merged soon or later (if it's not already!).

I am currently in Germany and on Wednesday 4th I fly and move to NYC -- so I'll be still on-and-off online this week. From next week I'll be online again :)

laura-barluzzi force-pushed the user_dashboard branch from a8ba10b to 9dd15f9 Compare August 16, 2017 00:27

laura-barluzzi assigned laura-barluzzi and clash99 Aug 16, 2017

laura-barluzzi requested review from oliverroick, seav and bjohare August 16, 2017 16:51

oliverroick requested review from alukach and removed request for oliverroick and bjohare August 21, 2017 09:35

amplifi added PR: needs review PR: needs testing labels Aug 22, 2017

laura-barluzzi force-pushed the user_dashboard branch from 9dd15f9 to aa624cf Compare August 22, 2017 15:54

alukach suggested changes Aug 22, 2017

View reviewed changes

laura-barluzzi force-pushed the user_dashboard branch 3 times, most recently from 07d76c6 to 444e28d Compare August 24, 2017 19:14

alukach suggested changes Aug 28, 2017

View reviewed changes

laura-barluzzi force-pushed the user_dashboard branch 2 times, most recently from 9777514 to 1cd4891 Compare September 3, 2017 08:40

laura-barluzzi force-pushed the user_dashboard branch from 1cd4891 to 601b516 Compare September 12, 2017 20:56

alukach suggested changes Sep 13, 2017

View reviewed changes

laura-barluzzi force-pushed the user_dashboard branch from 456a94e to 8f1baf5 Compare September 15, 2017 20:43

laura-barluzzi force-pushed the user_dashboard branch from 8f1baf5 to ab34b95 Compare September 15, 2017 21:31

alukach approved these changes Sep 19, 2017

View reviewed changes

laura-barluzzi and others added 9 commits September 26, 2017 22:18

updated and squashed all commits

57542d3

address 1st reviews from Anthony and display verbose values of langua…

a3fd2c8

…ge and measurement which are translatable

in progress changes or review queryset

77ee26c

attempt to address Anthony's suggestion by reducing queryset

bd4b441

fix errors and pass of user dashboard

ca0b17d

Sort output orgs, reduce queries, use operator helper to simplify code

e07bf53

Reorder test results to fit alphabetical output

728bfab

Restructure query to match style of other queries

a783862

clash99 force-pushed the user_dashboard branch from bceb411 to 90d86eb Compare September 27, 2017 03:20

Moved verification alert out of message area

ac81bdb

seav approved these changes Sep 27, 2017

View reviewed changes

seav added feature PR: approved and removed PR: needs review labels Sep 27, 2017

Missed translated string

a3d3ede

oliverroick merged commit 90ee1da into master Oct 2, 2017

oliverroick deleted the user_dashboard branch October 2, 2017 16:20

oliverroick mentioned this pull request Oct 26, 2017

Create test cases for user dashboard Cadasta/cadasta-test#49

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create User Dashboard #1721

Create User Dashboard #1721

laura-barluzzi commented Aug 15, 2017 •

edited

Loading

alukach left a comment

alukach Aug 22, 2017

laura-barluzzi Aug 22, 2017

alukach Aug 22, 2017

laura-barluzzi commented Aug 22, 2017

alukach Aug 28, 2017

laura-barluzzi Aug 30, 2017

alukach Aug 28, 2017

laura-barluzzi Aug 30, 2017

laura-barluzzi Sep 12, 2017 •

edited

Loading

laura-barluzzi commented Sep 12, 2017

alukach left a comment •

edited

Loading

alukach Sep 13, 2017 •

edited

Loading

alukach Sep 13, 2017

laura-barluzzi Sep 13, 2017

alukach Sep 13, 2017

alukach commented Sep 13, 2017

alukach commented Sep 15, 2017

laura-barluzzi commented Sep 15, 2017 •

edited

Loading

alukach commented Sep 19, 2017 •

edited

Loading

clash99 commented Sep 26, 2017

clash99 commented Sep 26, 2017 •

edited

Loading

clash99 commented Sep 27, 2017 •

edited

Loading

seav commented Sep 27, 2017

seav commented Sep 27, 2017

seav commented Sep 27, 2017

clash99 commented Sep 27, 2017

clash99 commented Sep 27, 2017

seav commented Sep 27, 2017

clash99 commented Sep 28, 2017

laura-barluzzi commented Oct 2, 2017

Create User Dashboard #1721

Create User Dashboard #1721

Conversation

laura-barluzzi commented Aug 15, 2017 • edited Loading

Proposed changes in this pull request

Sample screenshot of Mario Rossi user dashboard

When should this PR be merged

Risks

Follow-up actions

Checklist (for reviewing)

General

Functionality

Code

Tests

Security

Documentation

alukach left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laura-barluzzi commented Aug 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laura-barluzzi Sep 12, 2017 • edited Loading

Choose a reason for hiding this comment

laura-barluzzi commented Sep 12, 2017

alukach left a comment • edited Loading

Choose a reason for hiding this comment

alukach Sep 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alukach commented Sep 13, 2017

alukach commented Sep 15, 2017

laura-barluzzi commented Sep 15, 2017 • edited Loading

alukach commented Sep 19, 2017 • edited Loading

clash99 commented Sep 26, 2017

clash99 commented Sep 26, 2017 • edited Loading

clash99 commented Sep 27, 2017 • edited Loading

seav commented Sep 27, 2017

seav commented Sep 27, 2017

seav commented Sep 27, 2017

clash99 commented Sep 27, 2017

clash99 commented Sep 27, 2017

seav commented Sep 27, 2017

clash99 commented Sep 28, 2017

laura-barluzzi commented Oct 2, 2017

laura-barluzzi commented Aug 15, 2017 •

edited

Loading

laura-barluzzi Sep 12, 2017 •

edited

Loading

alukach left a comment •

edited

Loading

alukach Sep 13, 2017 •

edited

Loading

laura-barluzzi commented Sep 15, 2017 •

edited

Loading

alukach commented Sep 19, 2017 •

edited

Loading

clash99 commented Sep 26, 2017 •

edited

Loading

clash99 commented Sep 27, 2017 •

edited

Loading