Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge application and project communication tabs #4349

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 10 additions & 28 deletions hypha/apply/activity/services.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from django.contrib.contenttypes.models import ContentType
from django.db.models import OuterRef, Subquery
from django.db.models import OuterRef, Q, Subquery
from django.db.models.functions import JSONObject
from django.utils import timezone

Expand Down Expand Up @@ -37,44 +37,26 @@ def edit_comment(activity: Activity, message: str) -> Activity:
return activity


def get_related_actions_for_user(obj, user):
"""Return Activity objects related to an object, esp. useful with
ApplicationSubmission and Project.

Args:
obj: instance of a model class
user: user who these actions are visible to.

Returns:
[`Activity`][hypha.apply.activity.models.Activity] queryset
"""
related_query = type(obj).activities.rel.related_query_name

return (
Activity.actions.filter(**{related_query: obj})
.select_related("user")
.prefetch_related(
"related_object",
)
.visible_to(user)
)


def get_related_comments_for_user(obj, user):
def get_related_activities_for_user(obj, user):
"""Return comments/communications related to an object, esp. useful with
ApplicationSubmission and Project.

Args:
obj: instance of a model class
obj: instance of either an [`ApplicationSubmission`][hypha.apply.funds.models.submissions.ApplicationSubmission] or [`Project`][hypha.apply.projects.models.project.Project].
user: user who these actions are visible to.

Returns:
[`Activity`][hypha.apply.activity.models.Activity] queryset
"""
related_query = type(obj).activities.rel.related_query_name
if hasattr(obj, "project") and obj.project:
source_filter = Q(submission=obj) | Q(project=obj.project)
if hasattr(obj, "submission") and obj.submission:
source_filter = Q(submission=obj.submission) | Q(project=obj)
else:
source_filter = Q(submission=obj)

queryset = (
Activity.objects.filter(**{related_query: obj})
Activity.objects.filter(source_filter)
.exclude(current=False)
.select_related("user")
.prefetch_related(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<section class="ml-4 max-w-3xl md:ml-20">

<div class="timeline">
{% for activity in comments %}
{% for activity in activities %}
{% if activity.type == "comment" %}
{% include "activity/ui/activity-comment-item.html" with activity=activity %}
{% else %}
Expand All @@ -25,7 +25,7 @@
{% endif %}
</div>

{% if not comments %}
{% if not activities %}
{% trans "No comments available" %}
{% endif %}
</section>
Expand Down
6 changes: 5 additions & 1 deletion hypha/apply/activity/templatetags/activity_tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,11 @@ def visibility_options(activity, user) -> str:
Returns:
A JSON string of visibility options
"""
submission_partner_list = activity.source.partners.all()
if hasattr(activity.source, "partners"):
submission_partner_list = activity.source.partners.all()
else:
submission_partner_list = activity.source.submission.partners.all()

choices = activity.visibility_choices_for(user, submission_partner_list)
return json.dumps(choices)

Expand Down
38 changes: 22 additions & 16 deletions hypha/apply/activity/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ def partial_comments(request, content_type: str, pk: int):

This view handles comments for both 'submission' and 'project' content types.
It checks the user's permissions and fetches the related comments for the user.
The comments are paginated and rendered in the 'comment_list' template.
The comments are paginated and rendered in the 'activity_list' template.

Args:
request (HttpRequest): The HTTP request object.
content_type (str): The type of content ('submission' or 'project').
pk (int): The primary key of the content object.

Returns:
HttpResponse: The rendered 'comment_list' template with the context data.
HttpResponse: The rendered 'activity_list' template with the context data.
"""
if content_type == "submission":
obj = get_object_or_404(ApplicationSubmission, pk=pk)
Expand All @@ -54,17 +54,17 @@ def partial_comments(request, content_type: str, pk: int):
)
editable = False if obj.status == "complete" else True
else:
return render(request, "activity/include/comment_list.html", {})
return render(request, "activity/include/activity_list.html", {})

qs = services.get_related_comments_for_user(obj, request.user)
qs = services.get_related_activities_for_user(obj, request.user)
page = Paginator(qs, per_page=10, orphans=5).page(request.GET.get("page", 1))

ctx = {
"page": page,
"comments": page.object_list,
"activities": page.object_list,
"editable": editable,
}
return render(request, "activity/include/comment_list.html", ctx)
return render(request, "activity/include/activity_list.html", ctx)


@login_required
Expand Down Expand Up @@ -98,16 +98,22 @@ class ActivityContextMixin:
"""Mixin to add related 'comments' of the current view's 'self.object'"""

def get_context_data(self, **kwargs):
extra = {
# Do not prefetch on the related_object__author as the models
# are not homogeneous and this will fail
"comments": services.get_related_comments_for_user(
self.object, self.request.user
),
"comments_count": services.get_comment_count(
self.object, self.request.user
),
}
# Do not prefetch on the related_object__author as the models
# are not homogeneous and this will fail
user = self.request.user
activities = services.get_related_activities_for_user(
self.object, self.request.user
)

# Comments for both projects and applications exist under the original application
if isinstance(self.object, ApplicationSubmission):
application_obj = self.object
else:
application_obj = self.object.submission

comments_count = services.get_comment_count(application_obj, user)

extra = {"activities": activities, "comments_count": comments_count}
return super().get_context_data(**extra, **kwargs)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,17 +247,6 @@ <h4 class="m-0 sr-only">{% trans "Add communication" %}</h4>
</div>
</div>
</div>

{# Tab 3 #}
<div class="tabs__content" id="tab-3">
<div
class="feed ms-3"
hx-get="{% url 'funds:submissions:partial-activities' object.id %}"
hx-trigger="intersect once"
>
<p>{% trans "Loading…" %}</p>
</div>
</div>
</div>
{% endblock %}

Expand Down
6 changes: 0 additions & 6 deletions hypha/apply/funds/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
partial_reviews_card,
partial_reviews_decisions,
partial_screening_card,
partial_submission_activities,
partial_submission_answers,
partial_submission_lead,
sub_menu_bulk_update_lead,
Expand Down Expand Up @@ -172,11 +171,6 @@
partial_submission_lead,
name="partial-get-lead",
),
path(
"partial/activities/",
partial_submission_activities,
name="partial-activities",
),
path("lead/update/", UpdateLeadView.as_view(), name="lead_update"),
path("archive/", htmx_archive_unarchive_submission, name="archive"),
path(
Expand Down
14 changes: 0 additions & 14 deletions hypha/apply/funds/views/partials.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
from wagtail.models import Page

from hypha.apply.activity.messaging import MESSAGES, messenger
from hypha.apply.activity.services import (
get_related_actions_for_user,
)
from hypha.apply.categories.models import MetaTerm, Option
from hypha.apply.funds.forms import BatchUpdateReviewersForm
from hypha.apply.funds.models.reviewer_role import ReviewerRole
Expand Down Expand Up @@ -225,17 +222,6 @@ def sub_menu_category_options(request):
return render(request, "submissions/submenu/category.html", ctx)


@login_required
@require_http_methods(["GET"])
def partial_submission_activities(request, pk):
submission = get_object_or_404(ApplicationSubmission, pk=pk)
has_permission(
"submission_view", request.user, object=submission, raise_exception=True
)
ctx = {"actions": get_related_actions_for_user(submission, request.user)}
return render(request, "activity/include/action_list.html", ctx)


@login_required
@require_http_methods(["GET"])
def partial_reviews_card(request: HttpRequest, pk: str) -> HttpResponse:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Generated by Django 4.2.17 on 2025-01-07 17:57

from django.db import migrations
from hypha.apply.activity.models import COMMENT


def migrate_project_comments_to_application(apps, schema_editor):
Activity = apps.get_model("activity", "Activity")
ContentType = apps.get_model("contenttypes", "ContentType")
Project = apps.get_model("application_projects", "Project")
ApplicationSubmission = apps.get_model("funds", "ApplicationSubmission")
project_type = ContentType.objects.get_for_model(Project)
application_type = ContentType.objects.get_for_model(ApplicationSubmission)
for comment in Activity.objects.filter(
type=COMMENT, source_content_type=project_type
):
application = Project.objects.get(id=comment.source_object_id).submission
comment.source_content_type = application_type
comment.source_object_id = application.id
comment.save()


class Migration(migrations.Migration):
dependencies = [
("application_projects", "0096_remove_invoicedeliverable_deliverable_and_more"),
]

operations = [migrations.RunPython(migrate_project_comments_to_application)]
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% extends "base-apply.html" %}

{% load i18n contract_tools static wagtailcore_tags approval_tools invoice_tools project_tags %}
{% load i18n static wagtailcore_tags approval_tools invoice_tools project_tags %}
{% load heroicons %}

{% block title %}{{ object.title }}{% endblock %}
Expand Down Expand Up @@ -208,31 +208,19 @@ <h5>{% trans "Project form approvals" %}</h5>
<div class="feed">
{% if not object.is_archive %}
<h4 class="m-0 sr-only">{% trans "Add communication" %}</h4>
{% include "activity/include/comment_form.html" %}
{% include "activity/include/comment_form.html" with action=object.submission.get_absolute_url %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works but feels a bit, un neat. You go to projects/p:n/#communications, submit the form and end up on submissions/s:n/#communications. Not that normal users look at the address bar.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice catch, I didn't even notice that was the case with the redirect. Do you think it should be handled in htmx so the redirect doesn't happen?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or does it make sense now to break out communications to its own view and url?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a good idea, I can explore that a bit

Copy link
Contributor Author

@wes-otf wes-otf Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another (more temporary) solution is to change up the get_success_url to resolve to the referrer URL:

So swapping

def get_success_url(self):
return self.object.source.get_absolute_url() + "#communications"

with

    def get_success_url(self):
        if (ref := self.request.META.get("HTTP_REFERER")) and (path := urlparse(ref).path):
            try:
                url_resolved = resolve(path)
                if url_resolved.url_name == "detail":
                    return f"{path}#communications"
            except Resolver404:
                # If it couldn't be resolved, use the default `get_absolute_url()`
                pass
        return f"{self.object.source.get_absolute_url()}#communications"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The redirect is not a big issue so I do not tink we need hack around it.

If creating a stand alone view/url is a bit big we can do it in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to break the PRs up, I can take a stab at breaking it out into it's own view

{% endif %}

<div
class="comments"
id="comment-feed"
hx-get="{% url 'activity:partial-comments' 'project' object.id %}"
hx-get="{% url 'activity:partial-comments' 'submission' object.submission.id %}"
hx-trigger="intersect once"
>
<p>{% trans "Loading…" %}</p>
</div>
</div>
</div>

{# Tab 3 #}
<div class="tabs__content" id="tab-3">
<div
class="feed ms-3"
hx-get="{% url 'apply:projects:partial-activities' object.id %}"
hx-trigger="intersect once"
>
{% comment %} Loaded using the htmx via alpine's custom event "open-tab-3"{% endcomment %}
<p>{% trans "Loading…" %}</p>
</div>
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can delete this part from applicationsubmission_detail.html as well. I think we might even clear out all partial-activities urls and partial_submission_activities() unless they are still used somewhere else.

</div>
{% endblock content %}

Expand Down
6 changes: 0 additions & 6 deletions hypha/apply/projects/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
partial_get_invoice_detail_actions,
partial_get_invoice_status,
partial_get_invoice_status_table,
partial_project_activities,
partial_project_lead,
partial_project_title,
partial_supporting_documents,
Expand All @@ -77,11 +76,6 @@
include(
[
path("", ProjectDetailView.as_view(), name="detail"),
path(
"partial/activities/",
partial_project_activities,
name="partial-activities",
),
path("partial/lead/", partial_project_lead, name="project_lead"),
path("partial/title/", partial_project_title, name="project_title"),
path("lead/update/", UpdateLeadView.as_view(), name="lead_update"),
Expand Down
2 changes: 0 additions & 2 deletions hypha/apply/projects/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
partial_get_invoice_detail_actions,
partial_get_invoice_status,
partial_get_invoice_status_table,
partial_project_activities,
partial_project_lead,
partial_project_title,
partial_supporting_documents,
Expand All @@ -62,7 +61,6 @@
)

__all__ = [
"partial_project_activities",
"partial_project_lead",
"partial_project_title",
"partial_supporting_documents",
Expand Down
13 changes: 0 additions & 13 deletions hypha/apply/projects/views/project_partials.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,11 @@
from django.views.decorators.http import require_GET

from hypha.apply.activity.models import Activity
from hypha.apply.activity.services import (
get_related_actions_for_user,
)
from hypha.apply.funds.utils import get_statuses_as_params

from ..constants import statuses_and_table_statuses_mapping
from ..models.payment import Invoice
from ..models.project import ContractDocumentCategory, DocumentCategory, Project
from ..permissions import has_permission
from ..utils import get_project_status_choices


Expand All @@ -38,15 +34,6 @@ def partial_project_title(request, pk):
)


@login_required
@require_GET
def partial_project_activities(request, pk):
project = get_object_or_404(Project, pk=pk)
has_permission("project_access", request.user, object=project, raise_exception=True)
ctx = {"actions": get_related_actions_for_user(project, request.user)}
return render(request, "activity/include/action_list.html", ctx)


@login_required
@require_GET
def partial_supporting_documents(request, pk):
Expand Down
Loading