Skip to content

Commit

Permalink
fixed pagination of project listing page (& cleaned up & refactored)
Browse files Browse the repository at this point in the history
* cleaned up project list view

* refactored / cleaned up domain function to list / filter public projects
  and began domain namespace for projects

* fixed pagination of project listing page -- no longer discards filters

  as discusssed in #54, the platform's listing pages all suffer from
  submitting filters via POST, and previously from a pagination template
  which discarded any GET query params besides its own page number.

  the first problem was resolved in previous clean-up of the project
  listing view. this change fixes the pagination template, globally.

  the net result is that only the pagination of the project listing page
  is fixed, for now. (but the other pages may now be fixed by switching to
  GETs, alone.)
  • Loading branch information
jesteria committed Sep 3, 2020
1 parent 6f1b526 commit 5712268
Show file tree
Hide file tree
Showing 8 changed files with 209 additions and 112 deletions.
2 changes: 2 additions & 0 deletions src/marketplace/domain/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

from .notifications import NotificationDomain
from .user import UserDomain
from .proj import ProjectDomain


marketplace = MarketplaceDomain = Namespace('marketplace')

MarketplaceDomain._add_(NotificationDomain)
MarketplaceDomain._add_(UserDomain)
MarketplaceDomain._add_(ProjectDomain)
124 changes: 74 additions & 50 deletions src/marketplace/domain/proj.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import itertools
import re

from django.db import IntegrityError, transaction
from django.utils import timezone
from django.core.exceptions import PermissionDenied
from datetime import date

from namespaces import Namespace

from ..models.proj import (
Project, ProjectStatus, ProjectRole, ProjRole, ProjectFollower, ProjectLog, ProjectLogType, ProjectLogSource, ProjectDiscussionChannel, ProjectComment,
ProjectTask, TaskStatus, TaskRole, ProjectTaskRole, ProjectTaskReview, VolunteerApplication,
Expand All @@ -21,62 +26,81 @@
from .notifications import NotificationDomain, NotificationService
from marketplace.authorization.common import ensure_user_has_permission


def filter_public_projects(query_set):
return query_set.exclude(status=ProjectStatus.DRAFT) \
.exclude(status=ProjectStatus.EXPIRED) \
.exclude(status=ProjectStatus.DELETED)
return (query_set.exclude(status=ProjectStatus.DRAFT)
.exclude(status=ProjectStatus.EXPIRED)
.exclude(status=ProjectStatus.DELETED))


# Namespace declaration #

# TODO: continue/extend experiment with Namespaces over *Services


# Project domain #

ProjectDomain = Namespace('project')


@ProjectDomain
def list_public_projects(projname=None,
orgname=None,
skills=None,
social_cause=None,
project_status=None):
# We could also add the projects that are non-public but that also belong
# to the organizations that the user is member of. Should that be added
# or should users access those projects through the page of their org?
projects = filter_public_projects(Project.objects.all())

if projname:
projects = projects.filter(name__icontains=projname)

if orgname:
projects = projects.filter(organization__name__icontains=orgname)

if skills:
for skill in re.split(r'[,\s]+', skills):
projects = projects.filter(projecttask__projecttaskrequirement__skill__name__icontains=skill)

if social_cause:
if isinstance(social_cause, str):
social_cause = (social_cause,)

social_causes = [social_cause_view_model_translation[sc] for sc in social_cause
if sc in social_cause_view_model_translation]
projects = projects.filter(projectsocialcause__social_cause__in=social_causes).distinct()

if project_status:
if isinstance(project_status, str):
project_status = (project_status,)

project_statuses = itertools.chain.from_iterable(
project_status_view_model_translation[ps] for ps in project_status
if ps in project_status_view_model_translation
)
projects = projects.filter(status__in=project_statuses).distinct()

# Here we'll make this method order by creation_date descending, rather than by name.
# It's only used by the project list view, which wants it this way.
#
# However, upon refactor, it *might* make sense to make this configurable by call argument,
# (and have the view indicate this preference), or omitted entirely (and left to the caller
# to apply `order_by()`).
#
# And, this module can either continue to insist on name ascending, or it looks like this
# could be safely moved to the model's Meta default.
#
return projects.distinct().order_by('-creation_date')


class ProjectService:

class ProjectService():
@staticmethod
def get_project(request_user, projid):
return Project.objects.filter(pk=projid).annotate(follower_count=Count('projectfollower')).first()

@staticmethod
def get_all_public_projects(request_user, search_config=None):
# We could also add the projects that are non-public but that also belong
# to the organizations that the user is member of. Should that be added
# or should users access those projects through the page of their org?
base_query = filter_public_projects(Project.objects.all())
if search_config:
if 'projname' in search_config:
base_query = base_query.filter(name__icontains=search_config['projname'])
if 'orgname' in search_config:
base_query = base_query.filter(organization__name__icontains=search_config['orgname'])
if 'skills' in search_config:
for skill_fragment in search_config['skills'].split():
base_query = base_query.filter(projecttask__projecttaskrequirement__skill__name__icontains=skill_fragment.strip())
if 'social_cause' in search_config:
sc = search_config['social_cause']
if isinstance(sc, str):
sc = [sc]
social_causes = []
for social_cause_from_view in sc:
social_causes.append(social_cause_view_model_translation[social_cause_from_view])
# base_query = base_query.filter(project_cause__in=social_causes)
base_query = base_query.filter(projectsocialcause__social_cause__in=social_causes).distinct()
if 'project_status' in search_config:
project_status_list = search_config['project_status']
if isinstance(project_status_list, str):
project_status_list = [project_status_list]
project_statuses = []
for project_status_from_view in project_status_list:
status_filter = project_status_view_model_translation[project_status_from_view]
project_statuses.extend(status_filter)
base_query = base_query.filter(status__in=project_statuses).distinct()

# Here we'll make this method order by creation_date descending, rather than by name.
# It's only used by the project list view, which wants it this way.
#
# However, upon refactor, it *might* make sense to make this configurable by call argument,
# (and have the view indicate this preference), or omitted entirely (and left to the caller
# to apply `order_by()`).
#
# And, this module can either continue to insist on name ascending, or it looks like this
# could be safely moved to the model's Meta default.
#
return base_query.distinct().order_by('-creation_date')


@staticmethod
def get_all_organization_projects(request_user, org):
return Project.objects.filter(organization=org).order_by('name')
Expand Down
1 change: 1 addition & 0 deletions src/marketplace/models/org.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def get_choices():
)

class Organization(models.Model):

name = models.CharField(
max_length=200,
verbose_name="Organization name",
Expand Down
22 changes: 12 additions & 10 deletions src/marketplace/templates/marketplace/components/pagination.html
Original file line number Diff line number Diff line change
@@ -1,39 +1,41 @@
{% if page_obj.has_other_pages %}
{% load params %}

<nav aria-label="Page navigation example">
{% if page_obj.has_other_pages %}
{% with pagename=pagename|default:'page' %}
<nav aria-label="Page navigation">
<ul class="pagination justify-content-center">
{% if page_obj.has_previous %}
<li class="page-item"><a class="page-link" href="{{ baseurl }}?{{ pagename|default:'page' }}=1"><i class="fa fa-angle-double-left" aria-hidden="true"></i></a></li>
<li class="page-item"><a class="page-link" href="{{ baseurl }}?{{ pagename|default:'page' }}={{ page_obj.previous_page_number }}"><i class="fa fa-angle-left" aria-hidden="true"></i></a></li>
<li class="page-item"><a class="page-link" href="{% set_params_path pagename 1 %}"><i class="fa fa-angle-double-left" aria-hidden="true"></i></a></li>
<li class="page-item"><a class="page-link" href="{% set_params_path pagename page_obj.previous_page_number %}"><i class="fa fa-angle-left" aria-hidden="true"></i></a></li>
{% else %}
<li class="page-item disabled"><a class="page-link disabled" href="#"><i class="fa fa-angle-double-left" aria-hidden="true"></i></a></li>
<li class="page-item disabled"><a class="page-link disabled" href="#"><i class="fa fa-angle-left" aria-hidden="true"></i></a></li>
{% endif %}

{% if page_obj.number|add:'-4' > 1 %}
<li class="page-item"><a class="page-link" href="{{ baseurl }}?{{ pagename|default:'page' }}={{ page_obj.number|add:'-5' }}">&hellip;</a></li>

<li class="page-item"><a class="page-link" href="{% set_params_path pagename page_obj.number|add:'-5' %}">&hellip;</a></li>
{% endif %}

{% for i in page_obj.paginator.page_range %}
{% if page_obj.number == i %}
<li class="page-item active"><span class="page-link">{{ i }} <span class="sr-only">(current)</span></span></li>
{% elif i > page_obj.number|add:'-5' and i < page_obj.number|add:'5' %}
<li class="page-item"><a class="page-link" href="{{ baseurl }}?{{ pagename|default:'page' }}={{ i }}">{{ i }}</a></li>
<li class="page-item"><a class="page-link" href="{% set_params_path pagename i %}">{{ i }}</a></li>
{% endif %}
{% endfor %}

{% if page_obj.paginator.num_pages > page_obj.number|add:'4' %}
<li class="page-item"><a class="page-link" href="{{ baseurl }}?{{ pagename|default:'page' }}={{ page_obj.number|add:'5' }}">&hellip;</a></li>
<li class="page-item"><a class="page-link" href="{% set_params_path pagename page_obj.number|add:'5' %}">&hellip;</a></li>
{% endif %}

{% if page_obj.has_next %}
<li class="page-item"><a class="page-link" href="{{ baseurl }}?{{ pagename|default:'page' }}={{ page_obj.next_page_number }}"><i class="fa fa-angle-right" aria-hidden="true"></i></a></li>
<li class="page-item"><a class="page-link" href="{{ baseurl }}?{{ pagename|default:'page' }}={{ page_obj.paginator.num_pages }}"><i class="fa fa-angle-double-right" aria-hidden="true"></i></a></li>
<li class="page-item"><a class="page-link" href="{% set_params_path pagename page_obj.next_page_number %}"><i class="fa fa-angle-right" aria-hidden="true"></i></a></li>
<li class="page-item"><a class="page-link" href="{% set_params_path pagename page_obj.paginator.num_pages %}"><i class="fa fa-angle-double-right" aria-hidden="true"></i></a></li>
{% else %}
<li class="page-item disabled"><a class="page-link disabled" href="#"><i class="fa fa-angle-right" aria-hidden="true"></i></a></li>
<li class="page-item disabled"><a class="page-link disabled" href="#"><i class="fa fa-angle-double-right" aria-hidden="true"></i></a></li>
{% endif %}
</ul>
</nav>
{% endwith %}
{% endif %}
8 changes: 4 additions & 4 deletions src/marketplace/templates/marketplace/proj_list.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
<div class="row">
<div class="col-lg-3">
<form id="projlist">
{% csrf_token %}
<h4>Filter results</h4>
<label for="projname" class="col-lg-12 pl-0 pr-0">
Project name
Expand Down Expand Up @@ -36,9 +35,10 @@ <h4>Filter results</h4>
{% include 'marketplace/components/filter_checkbox.html' with field_name='projectstatus' field_value='in_progress' field_text='In progress' is_checked=checked_project_fields.in_progress %}
{% include 'marketplace/components/filter_checkbox.html' with field_name='projectstatus' field_value='completed' field_text='Completed' is_checked=checked_project_fields.completed %}
</fieldset>

<button type="submit"
form="projlist"
formmethod="post"
formmethod="get"
formaction="{% url 'marketplace:proj_list' %}"
class="btn btn-success col-lg-12 mt-3">
<i class="material-icons" style="vertical-align: middle">filter_list</i>
Expand Down Expand Up @@ -86,8 +86,8 @@ <h4>Filter results</h4>
</tbody>
</table>
</div>
{% url 'marketplace:proj_list' as proj_list_url %}
{% include 'marketplace/components/pagination.html' with baseurl=proj_list_url page_obj=proj_list %}

{% include 'marketplace/components/pagination.html' with page_obj=proj_list %}

{% else %}
<p>No projects found.</p>
Expand Down
55 changes: 55 additions & 0 deletions src/marketplace/templatetags/params.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
"""template tags interacting with the request's query parameters"""
from django import template


register = template.Library()



@register.simple_tag(takes_context=True)
def set_params(context, *interleaved, **pairs):
"""construct a copy of the current request's query parameters,
updated with the given parameters.
parameter keys and values may be specified either as named keyword
argument pairs:
set_params query='cookie' page=2
and/or as interleaved keys and values (permitting the template
language to specify variable keys):
set_params querykey 'cookie' pagekey nextpage
(which might evaluate as):
set_params 'query' 'cookie' 'page' 2
"""
request = context['request']
params = request.GET.copy()

while interleaved:
try:
(key, value, *interleaved) = interleaved
except ValueError:
raise TypeError('incorrect number of arguments for interleaved parameter pairs')

params[key] = value

params.update(pairs)

return params.urlencode()


@register.simple_tag(takes_context=True)
def set_params_path(context, *interleaved, **pairs):
"""construct the current full path (including query), updated by the
given parameters.
(See: ``set_params``.)
"""
request = context['request']
params = set_params(context, *interleaved, **pairs)
return request.path + '?' + params
6 changes: 3 additions & 3 deletions src/marketplace/tests/domain/test_proj.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def setUp(self):
self.project = example_project()

def test_create_project(self):
self.assertEqual(list(ProjectService.get_all_public_projects(self.owner_user, None)), [])
self.assertEqual(list(marketplace.project.list_public_projects()), [])
self.assertEqual(list(ProjectService.get_all_organization_projects(self.owner_user, self.organization)), [])
self.assertEqual(list(ProjectService.get_organization_public_projects(self.owner_user, self.organization)), [])
self.assertEqual(ProjectService.get_project(self.owner_user, 1), None)
Expand All @@ -98,7 +98,7 @@ def test_create_project(self):
OrganizationService.create_project(self.owner_user, self.organization.id, self.project)

projects_list = [self.project]
self.assertEqual(list(ProjectService.get_all_public_projects(self.owner_user, None)), [])
self.assertEqual(list(marketplace.project.list_public_projects()), [])
self.assertEqual(list(ProjectService.get_all_organization_projects(self.owner_user, self.organization)), projects_list)
self.assertEqual(list(ProjectService.get_organization_public_projects(self.owner_user, self.organization)), [])
self.assertEqual(ProjectService.get_featured_project(), None)
Expand All @@ -122,7 +122,7 @@ def test_create_project(self):
lambda x: ProjectService.publish_project(x, self.project.id, self.project))
ProjectService.publish_project(self.owner_user, self.project.id, self.project)

self.assertEqual(list(ProjectService.get_all_public_projects(self.owner_user, None)), [self.project])
self.assertEqual(list(marketplace.project.list_public_projects()), [self.project])
self.assertEqual(list(ProjectService.get_all_organization_projects(self.owner_user, self.organization)), projects_list)
self.assertEqual(list(ProjectService.get_organization_public_projects(self.owner_user, self.organization)), projects_list)
self.assertEqual(ProjectService.get_featured_project(), self.project)
Expand Down
Loading

0 comments on commit 5712268

Please sign in to comment.