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

LTD-5817-add-application-history #2343

Merged
merged 4 commits into from
Jan 31, 2025
Merged

Conversation

depsiatwal
Copy link
Contributor

https://uktrade.atlassian.net/browse/LTD-5817

Add a new view on the exporter application details.

This utilises a new endpoint which extracts the history

uktrade/lite-api#2388

Copy link
Contributor

@currycoder currycoder left a comment

Choose a reason for hiding this comment

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

One comment which I'll leave with you. Otherwise looking good

url = reverse("applications:application", kwargs={"pk": application["id"]})
response = authorized_client.get(url)
soup = BeautifulSoup(response.content, "html.parser")

products_table = soup.find_all("table", attrs={"class": "govuk-table"})[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than relying on finding the second table, can we make this lookup more bulletproof with a unique CSS id or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added id and to search by id

@depsiatwal depsiatwal force-pushed the LTD-5817-add-application-history branch from 133570b to 47c2de7 Compare January 29, 2025 11:54
@depsiatwal depsiatwal force-pushed the LTD-5817-add-application-history branch from 47c2de7 to 6253ff6 Compare January 29, 2025 17:22
Copy link
Contributor

@Tllew Tllew left a comment

Choose a reason for hiding this comment

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

Looks good

@depsiatwal depsiatwal force-pushed the LTD-5817-add-application-history branch from 5dcbe43 to 4d00a35 Compare January 30, 2025 18:02
Comment on lines 14 to 36
{% if prev_application.status.status != "draft" %}
<td class="govuk-table__cell">
{% if prev_application.id == application.id %}
{{ prev_application.reference_code }}
{% else %}
<a href="{% url 'applications:application' prev_application.id %}">{{ prev_application.reference_code }}</a>
{% endif %}
</td>
<td class="govuk-table__cell">{{ prev_application.submitted_at|str_date }}</td>
<td class="govuk-table__cell" id="label-application-status-{{ forloop.counter }}"> <div class="govuk-tag govuk-tag--grey govuk-!-margin-0">{{ prev_application.status.status_display }}</div></td>
<td class="govuk-table__cell">
{% if prev_application.ecju_query_count > 0 %}
<a href="{% url 'applications:application' prev_application.id 'ecju-queries' %}">{{ prev_application.ecju_query_count }}</a>
{% endif %}
</td>
{% else %}
<td class="govuk-table__cell">
<a href="{% url 'applications:task_list' prev_application.id %}" >{{ prev_application.status.status_display|capfirst }}</a>
</td>
<td class="govuk-table__cell"/>
<td class="govuk-table__cell" id="label-application-status-{{ forloop.counter }}"> <div class="govuk-tag govuk-tag--grey govuk-!-margin-0">{{ prev_application.status.status_display }}</div></td>
<td class="govuk-table__cell"/>
{% endif %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe flipping round the if statement will make this slightly easier to understand e.g.

{% if prev_application.status.status == "draft" %}
    // draft table
{% else %}
    // non-draft table
{% endif %}

Copy link
Contributor

@currycoder currycoder left a comment

Choose a reason for hiding this comment

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

One comment which I'll leave with you

@depsiatwal depsiatwal force-pushed the LTD-5817-add-application-history branch from 83507f3 to 0e5981c Compare January 31, 2025 08:53
@depsiatwal depsiatwal merged commit 6b86422 into dev Jan 31, 2025
12 checks passed
@depsiatwal depsiatwal deleted the LTD-5817-add-application-history branch January 31, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants