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.

Sign up for GitHub

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/diff and other minor issues. #711

Merged
merged 21 commits into from
Oct 22, 2024
Merged

Fix/diff and other minor issues. #711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
8093d5e
fix: update the diff template
EdoStorm96 Sep 26, 2024
cd09962
fix: make sure StudyDesign also deletes study types
EdoStorm96 Sep 26, 2024
06a6da9
fix: small bug on task_create
EdoStorm96 Sep 26, 2024
f9b095b
Merge branch 'major/4' into fix/diff
EdoStorm96 Sep 30, 2024
82863a6
feat: dumb css implementation on compare_documents.html
EdoStorm96 Sep 30, 2024
690ae5a
fix: simplify diff css and put in own diff.css file
EdoStorm96 Oct 1, 2024
da35b00
fix: use Session.repeats too for Session.net_duration()
EdoStorm96 Oct 1, 2024
7d45799
fix: reinsert diff-ignore
EdoStorm96 Oct 1, 2024
0eed19e
style: back to black
EdoStorm96 Oct 1, 2024
13f4a88
fix: change default view for compare_documents to split
EdoStorm96 Oct 7, 2024
5bc4b70
fix: make compare_documents extend fetc_base
EdoStorm96 Oct 7, 2024
ab1ce42
fix: improve styling on compare_documents.html
EdoStorm96 Oct 8, 2024
87c1c26
style: djlint
EdoStorm96 Oct 8, 2024
aef04eb
fix: undo self.repeats in Session.net_duration()
EdoStorm96 Oct 10, 2024
54118f9
Merge branch 'major/4' into fix/diff
EdoStorm96 Oct 10, 2024
0d9376d
fix: typo
EdoStorm96 Oct 10, 2024
565bff2
fix: replace th with td on row.verbose_name in diff
EdoStorm96 Oct 10, 2024
aa21af3
fix: implement get property methods for study_types, to not have to d…
EdoStorm96 Oct 10, 2024
1a1224a
style: style
EdoStorm96 Oct 10, 2024
8167d23
fix: correct title variable proposal_confirmation.html
EdoStorm96 Oct 10, 2024
c320001
Merge branch 'major/4' into fix/diff
EdoStorm96 Oct 22, 2024
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
196 changes: 96 additions & 100 deletions locale/en/LC_MESSAGES/django.po
View file
Open in desktop

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions main/static/main/diff.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
.diff del {
text-decoration: none;
background-color: #ff6565;
}

.diff ins {
text-decoration: none;
background-color: #73ff73;
}
72 changes: 38 additions & 34 deletions proposals/templates/proposals/compare_documents.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{% extends 'base/base.html' %}
{% extends 'base/fetc_base.html' %}

{% load static %}
{% load i18n %}
Expand All @@ -9,6 +9,7 @@
{% endblock %}

{% block html_head %}
<link rel="stylesheet" type="text/css" href="{% static 'main/diff.css' %}">
<script src="{% static 'main/js/htmldiff.js' %}"></script>
<script defer>
$(function () {
Expand Down Expand Up @@ -50,48 +51,51 @@ <h2>{% trans 'Vergelijk documenten' %}</h2>
<p>
{% blocktrans trimmed %}
Hier kan je twee versies van een document vergelijken. Standaard
geeft hij een <i>gecombineerde</i> versie weer, waarbij tekst
geeft hij een <i>gesplitste</i> versie weer, waarbij tekst
die verwijderd is in het rood is gemarkeerd en nieuwe tekst
in het groen is gemarkeerd.
{% endblocktrans %}
</p>
<p>
{% blocktrans trimmed %}
Je kan ook de bestanden naast elkaar bekijken, met dezelfde
markeringen. Klik hiervoor op 'Bekijk apart'.
Je kan ook de bestanden gecombineerd bekijken, met dezelfde
markeringen. Klik hiervoor op 'Bekijk gecombineerd'.
{% endblocktrans %}
</p>
<h4>
<strong>{% trans "Je vergelijkt" %}:</strong>
{% describe_file new_file %}
</h4>
<div class="warning" style="display: none; clear: both;">
{% trans 'Er zijn geen wijzigingen gevonden in dit document!' %}
<div class="d-flex justify-content-between align-items-center">
<h4>
<strong>{% trans "Je vergelijkt" %}:</strong>
{% describe_file new_file %}
</h4>
<button id="button-split"
class="float-end btn btn-secondary"
style="display: none">{% trans 'Bekijk apart' %}</button>
<button id="button-combined" class="float-end btn btn-secondary">{% trans 'Bekijk gecombineerd' %}</button>
</div>
<button id="button-split" class="float-end">{% trans 'Bekijk apart' %}</button>
<button id="button-combined" style="display: none" class="float-end">{% trans 'Bekijk gecombineerd' %}</button>
<div class="warning alert alert-info mt-4"
style="display: none;
clear: both">{% trans 'Er zijn geen wijzigingen gevonden in dit document!' %}</div>
</div>
</div>
<div class="uu-container document-compare">
<div class="col-6">
<p>
<strong>{% trans "Oud" %}:</strong>
<a href="{{ old_file.url }}" download="{{ old_file.name }}">{{ old_name }}</a>
</p>
</div>
<div class="col-6">
<p>
<strong>{% trans "Nieuw" %}:</strong>
<a href="{{ new_file.url }}" download="{{ new_file.name }}">{{ new_name }}</a>
</p>
<div class="uu-container document-compare">
<div class="col-6">
<p>
<strong>{% trans "Oud" %}:</strong>
<a href="{{ old_file.url }}" download="{{ old_file.name }}">{{ old_name }}</a>
</p>
</div>
<div class="col-6">
<p>
<strong>{% trans "Nieuw" %}:</strong>
<a href="{{ new_file.url }}" download="{{ new_file.name }}">{{ new_name }}</a>
</p>
</div>
</div>
</div>
<div class="uu-container compare">
<div class="col-12 text-center mt-5 mb-5" id="loading-icon">
<img src="{% static 'main/images/loading.gif' %}" />
<div class="uu-container compare">
<div class="col-12 text-center mt-5 mb-5" id="loading-icon">
<img src="{% static 'main/images/loading.gif' %}" />
</div>
<div class="col-6 split_view diff" id="old_text">{{ old_text|linebreaks }}</div>
<div class="col-6 split_view diff" id="new_text">{{ new_text|linebreaks }}</div>
<div class="col-12 diff" id="combined_text" style="display: none"></div>
</div>
<div style="display: none" class="col-6 split_view" id="old_text">{{ old_text|linebreaks }}</div>
<div style="display: none" class="col-6 split_view" id="new_text">{{ new_text|linebreaks }}</div>
<div class="col-12" id="combined_text"></div>
</div>
{% endblock %}
{% endblock %}
2 changes: 1 addition & 1 deletion proposals/templates/proposals/proposal_confirmation.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
{% block pre-form-text %}
<h2>{% trans "Bevestigingsbrief versturen" %}</h2>
<p>
{% blocktrans trimmed with title=review.proposal.title %}
{% blocktrans trimmed with title=proposal.title %}
Geef hieronder aan wanneer de bevestigingsbrief voor de aanvraag <em>{{ title }}</em> is verstuurd.
{% endblocktrans %}
</p>
Expand Down
1 change: 1 addition & 0 deletions proposals/templates/proposals/proposal_diff.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
{% endblock %}

{% block html_head %}
<link rel="stylesheet" type="text/css" href="{% static 'main/diff.css' %}">
<script src="{% static 'main/js/htmldiff.js' %}"></script>
<script>
$(function () {
Expand Down
83 changes: 44 additions & 39 deletions proposals/templates/proposals/table_with_header_diff.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,48 +1,53 @@
{% load diff_tags %}
{% load i18n %}
{% load static %}

{% if section_title %}
<h2 class="mt-5 mb-1">{% trans section_title %}</h2>
{% endif %}
{% if sub_title %}<h3 class='mt-3'>{{ sub_title }}</h3>{% endif %}
<table class="proposals-diff ">
<tr>
<th class="proposals-diff-question"></th>
<th class="proposals-diff-answer">{% trans "Vorige aanvraag" %}</th>
<th class="proposals-diff-answer">{% trans "Huidige aanvraag" %}</th>
</tr>
{% if warning %}
{% for row in rows|slice:":1" %}
<tr class="diff-ignore">
<th>{{ row.verbose_name }}</th>
{# If the first object is missing, place a warning on the first TD #}
{% if missing_object == 'old' %}
<td rowspan='{{ rows|length }}'>
<div class='warning'>{{ warning }}</div>
</td>
<table class="table">
<thead>
<tr>
<th scope="col" width="30%"></th>
<th scope="col" width="35%">{% trans "Vorige aanvraag" %}</th>
<th scope="col" width="35%">{% trans "Huidige aanvraag" %}</th>
</tr>
</thead>
<tbody>
{% if warning %}
{% for row in rows|slice:":1" %}
<tr class="diff-ignore">
<td>{{ row.verbose_name }}</td>
{# If the first object is missing, place a warning on the first TD #}
{% if missing_object == 'old' %}
<td rowspan='{{ rows|length }}' class="align-middle">
<div class='alert alert-info'>{{ warning }}</div>
</td>
<td>{{ row.value }}</td>
{# Otherwise, we're missing the second one, thus we place the warning on the second #}
{% elif missing_object == 'new' %}
<td>{{ row.value }}</td>
<td rowspan='{{ rows|length }}' class="align-middle">
<div class='alert alert-info'>{{ warning }}</div>
</td>
{% endif %}
</tr>
{% endfor %}
{% for row in rows|slice:"1:" %}
<tr class="diff-ignore">
<td>{{ row.verbose_name }}</td>
<td>{{ row.value }}</td>
{# Otherwise, we're missing the second one, thus we place the warning on the second #}
{% elif missing_object == 'new' %}
<td>{{ row.value }}</td>
<td rowspan='{{ rows|length }}'>
<div class='warning'>{{ warning }}</div>
</td>
{% endif %}
</tr>
{% endfor %}
{% for row in rows|slice:"1:" %}
<tr class="diff-ignore">
<th>{{ row.verbose_name }}</th>
<td>{{ row.value }}</td>
</tr>
{% endfor %}
{% else %}
{% for row in rows %}
<tr>
<th>{{ row.verbose_name }}</th>
<td>{{ row.old_value }}</td>
<td>{{ row.new_value }}</td>
</tr>
{% endfor %}
{% endif %}
</tr>
{% endfor %}
{% else %}
{% for row in rows %}
<tr class="diff">
<td>{{ row.verbose_name }}</td>
<td>{{ row.old_value }}</td>
<td>{{ row.new_value }}</td>
</tr>
{% endfor %}
{% endif %}
</tbody>
</table>
8 changes: 5 additions & 3 deletions proposals/utils/pdf_diff_logic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1145,7 +1145,9 @@ def create_context_diff(context, old_proposal, new_proposal):
or new_study is not None
and new_study.has_intervention
):
interventions = get_all_related(both_studies, "intervention")
interventions = get_all_related(
both_studies, "get_intervention"
)

sections.append(
DiffSection(
Expand All @@ -1159,7 +1161,7 @@ def create_context_diff(context, old_proposal, new_proposal):
or new_study is not None
and new_study.has_observation
):
observations = get_all_related(both_studies, "observation")
observations = get_all_related(both_studies, "get_observation")

sections.append(
DiffSection(
Expand All @@ -1174,7 +1176,7 @@ def create_context_diff(context, old_proposal, new_proposal):
and new_study.has_sessions
):
old_sessions_set, new_sessions_set = get_all_related_set(
both_studies, "session_set"
both_studies, "get_sessions"
)

for both_sessions in zip_equalize_lists(
Expand Down
15 changes: 15 additions & 0 deletions studies/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,21 @@ class Meta:
ordering = ["order"]
unique_together = ("proposal", "order")

@property
def get_intervention(self):
if self.has_intervention and hasattr(self, "intervention"):
return self.intervention

@property
def get_observation(self):
if self.has_observation and hasattr(self, "observation"):
return self.observation

@property
def get_sessions(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

This method will either return a list of sessions, or implicitly return None. I would prefer if it returned an empty list if the proposal has no sessions.

If we do that then for session in proposal.get_sessions(): will just do nothing, rather than produce an error. And the empty list will still be falsey for checking if a proposal has sessions.

Copy link
Contributor

Choose a reason for hiding this comment

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

It will actually return a QS or a list in that case, but at least they're both iterable.

An empty QS is still falsey thankfully.

if self.has_sessions:
return self.session_set.all()

@property
def sessions_number(self):
return self.session_set.count()
Expand Down
3 changes: 3 additions & 0 deletions tasks/views/task_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ def get_session(self):
"""Retrieves the Study from the pk kwarg"""
return Session.objects.get(pk=self.kwargs["pk"])

def get_proposal(self):
return self.get_session().study.proposal


class TaskUpdate(TaskMixin, UpdateView):
pass
Expand Down
Loading