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

Fix Responses and History Section Performance #582

Merged
merged 3 commits into from
May 30, 2024
Merged
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
19 changes: 18 additions & 1 deletion app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1077,7 +1077,7 @@ class Events(db.Model):

__tablename__ = "events"
id = db.Column(db.Integer, primary_key=True)
request_id = db.Column(db.String(19), db.ForeignKey("requests.id"))
request_id = db.Column(db.String(19), db.ForeignKey("requests.id"), index=True)
user_guid = db.Column(db.String(64))
response_id = db.Column(db.Integer, db.ForeignKey("responses.id"))
type = db.Column(db.String(64))
Expand Down Expand Up @@ -1367,6 +1367,23 @@ def communication_method_type(self):
else response_type.EMAIL
)

@property
def is_determination_letter(self):
"""
Determine if a response is a determination letter or not.

Letters created as part of a Determination will appear in the CommunicationMethod table
response_id will be the Determination response and method_id will be the corresponding Letter response.

Letters created using the Generate Letter action will not appear in the CommunicationMethod table.
"""
method_id = CommunicationMethods.query.filter_by(
method_id=self.id
).one_or_none()
if method_id is None:
return False
return True

@property
def event_timestamp(self):
"""
Expand Down
8 changes: 5 additions & 3 deletions app/request/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,18 +218,21 @@ def get_request_responses():
# If the user is an agency user assigned to the request, all responses can be retrieved.
responses = Responses.query.filter(
Responses.request_id == current_request.id,
~Responses.id.in_([cm.method_id for cm in CommunicationMethods.query.all()]),
Responses.type != response_type.EMAIL,
Responses.deleted == False
).order_by(
desc(Responses.date_modified)
).all()
# Remove determination letters, done post Responses query to reduce performance overheard.
# Slow performance when doing a nested query with Responses and CommunicationMethods.
for index, response in enumerate(responses):
if response.type == response_type.LETTER and response.is_determination_letter:
del responses[index]
elif current_user == current_request.requester:
# If the user is the requester, then only responses that are "Release and Private" or "Release and Public"
# can be retrieved.
responses = Responses.query.filter(
Responses.request_id == current_request.id,
~Responses.id.in_([cm.method_id for cm in CommunicationMethods.query.all()]),
Responses.type != response_type.EMAIL,
Responses.deleted == False,
Responses.privacy.in_([response_privacy.RELEASE_AND_PRIVATE, response_privacy.RELEASE_AND_PUBLIC])
Expand All @@ -242,7 +245,6 @@ def get_request_responses():
# "Release and Public" whose release date is not in the future can be retrieved.
responses = Responses.query.filter(
Responses.request_id == current_request.id,
~Responses.id.in_([cm.method_id for cm in CommunicationMethods.query.all()]),
Responses.type != response_type.EMAIL,
Responses.deleted == False,
Responses.privacy.in_([response_privacy.RELEASE_AND_PUBLIC]),
Expand Down
28 changes: 13 additions & 15 deletions app/templates/request/responses/modal_body/notes.html
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
<div>
Created By:
{% if current_user == response.creator %}
You
{% elif current_user.is_agency %}
{{ response.creator.name }}
{% else %}
Agency
{% endif %}
</div>
<br>

{#<div>#}
{# Created By:#}
{# {% if current_user == response.creator %}#}
{# You#}
{# {% elif current_user.is_agency %}#}
{# {{ response.creator.name }}#}
{# {% else %}#}
{# Agency#}
{# {% endif %}#}
{#</div>#}
{#<br>#}
{% if (edit_response_permission or edit_privacy_response_permission) and is_editable
and current_request.status != request_status.CLOSED %}
<form id="note-update-{{ response.id }}" class="note-update note-form"
Expand Down Expand Up @@ -51,8 +50,7 @@

<script>
{% if not ((edit_response_permission or edit_privacy_response_permission) and is_editable and current_request.status != request_status.CLOSED) or not edit_response_permission %}
let responseModalBody = $('#' + '{{ modal_html_id }}');
responseModalBody.html({{ response.content | tojson }});
responseModalBody.html(responseModalBody.text());
$('#' + '{{ modal_html_id }}').html({{ response.content | tojson }});
$('#' + '{{ modal_html_id }}').html($('#' + '{{ modal_html_id }}').text());
{% endif %}
</script>
2 changes: 1 addition & 1 deletion app/templates/request/responses/row.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

{% if current_user.is_agency %}
<div class="col-md-5 text-right">
{{ moment(response.event_timestamp).format('dddd, MM/DD/YYYY [at] h:mm A') }}
{{ moment(response.date_modified).format('dddd, MM/DD/YYYY [at] h:mm A') }}
</div>
<div class="row">
<div class="col-md-10 metadata-preview" id="{{ row_html_id }}"></div>
Expand Down
26 changes: 26 additions & 0 deletions migrations/versions/1f95398557cd_events_table_index.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
"""Events table index

Revision ID: 1f95398557cd
Revises: 0fddd88d0a1d
Create Date: 2024-05-23 22:38:42.760122

"""

# revision identifiers, used by Alembic.
revision = '1f95398557cd'
down_revision = '0fddd88d0a1d'

from alembic import op
import sqlalchemy as sa
from sqlalchemy.dialects import postgresql

def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.create_index(op.f('ix_events_request_id'), 'events', ['request_id'], unique=False)
# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_index(op.f('ix_events_request_id'), table_name='events')
# ### end Alembic commands ###