-
Notifications
You must be signed in to change notification settings - Fork 18
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
Issue #2595: update domain request view only pages - [ZA] #2753
Changes from 14 commits
01ce2fb
e5a011c
5be9f3f
b5b2d69
eb38504
ef0e1c0
82c5d7f
ac9edec
b31c7cd
fe8a680
e5f26e5
1248954
484a4b4
11c7268
999ed2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,33 +8,30 @@ | |
{% block content %} | ||
<main id="main-content" class="grid-container"> | ||
<div class="grid-col desktop:grid-offset-2 desktop:grid-col-8"> | ||
{% comment %} | ||
TODO: Uncomment in #2596 | ||
{% if portfolio %} | ||
{% url 'domain-requests' as url %} | ||
<nav class="usa-breadcrumb padding-top-0" aria-label="Domain request breadcrumb"> | ||
<ol class="usa-breadcrumb__list"> | ||
<li class="usa-breadcrumb__list-item"> | ||
{% else %} | ||
{% url 'home' as url %} | ||
{% endif %} | ||
<nav class="usa-breadcrumb padding-top-0" aria-label="Domain request breadcrumb"> | ||
<ol class="usa-breadcrumb__list"> | ||
<li class="usa-breadcrumb__list-item"> | ||
{% if portfolio %} | ||
<a href="{{ url }}" class="usa-breadcrumb__link"><span>Domain requests</span></a> | ||
</li> | ||
<li class="usa-breadcrumb__list-item usa-current" aria-current="page"> | ||
<span>{{ DomainRequest.requested_domain.name }}</span | ||
> | ||
</li> | ||
</ol> | ||
</nav> | ||
{% else %}{% endcomment %} | ||
{% url 'home' as url %} | ||
<a href="{{ url }}" class="breadcrumb__back"> | ||
<svg class="usa-icon" aria-hidden="true" focusable="false" role="img"> | ||
<use xlink:href="{% static 'img/sprite.svg' %}#arrow_back"></use> | ||
</svg> | ||
|
||
<p class="margin-left-05 margin-top-0 margin-bottom-0 line-height-sans-1"> | ||
Back to manage your domains | ||
</p> | ||
</a> | ||
{% comment %} {% endif %}{% endcomment %} | ||
{% else %} | ||
<a href="{{ url }}" class="usa-breadcrumb__link"><span>Manage your domains</span></a> | ||
{% endif %} | ||
</li> | ||
<li class="usa-breadcrumb__list-item usa-current" aria-current="page"> | ||
{% if not DomainRequest.requested_domain and DomainRequest.status == DomainRequest.DomainRequestStatus.STARTED %} | ||
<span>New domain request</span> | ||
{% else %} | ||
<span>{{ DomainRequest.requested_domain.name }}</span> | ||
{% endif %} | ||
</li> | ||
</ol> | ||
</nav> | ||
|
||
<h1>Domain request for {{ DomainRequest.requested_domain.name }}</h1> | ||
<div | ||
class="usa-summary-box dotgov-status-box margin-top-3 padding-left-2" | ||
|
@@ -48,25 +45,71 @@ <h1>Domain request for {{ DomainRequest.requested_domain.name }}</h1> | |
<span class="text-bold text-primary-darker"> | ||
Status: | ||
</span> | ||
{% if DomainRequest.status == 'approved' %} Approved | ||
{% elif DomainRequest.status == 'in review' %} In review | ||
{% elif DomainRequest.status == 'rejected' %} Rejected | ||
{% elif DomainRequest.status == 'submitted' %} Submitted | ||
{% elif DomainRequest.status == 'ineligible' %} Ineligible | ||
{% else %}ERROR Please contact technical support/dev | ||
{% endif %} | ||
{{ DomainRequest.get_status_display|default:"ERROR Please contact technical support/dev" }} | ||
</p> | ||
</div> | ||
</div> | ||
<br> | ||
<p><b class="review__step__name">Last updated:</b> {{DomainRequest.updated_at|date:"F j, Y"}}</p> | ||
|
||
|
||
{% with statuses=DomainRequest.DomainRequestStatus last_submitted=DomainRequest.last_submitted_date|date:"F j, Y" first_submitted=DomainRequest.first_submitted_date|date:"F j, Y" last_status_update=DomainRequest.last_status_update|date:"F j, Y" %} | ||
{% comment %} | ||
These are intentionally seperated this way. | ||
There is some code repetition, but it gives us more flexibility rather than a dense reduction. | ||
Leave it this way until we've solidified our requirements. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just checking if there ended up being any requirements set or any tickets to set these requirements on the way we display status and if we yes, if we should reference them here! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No tickets in particular that I am aware of! That is a good callout though and I'm glad you thought of that. These pages are still undergoing design review + we are fleshing out the org feature. Personally, I think these dates may shift around - or we may decide to add some more in the future. That was my thinking, at least |
||
{% endcomment %} | ||
{% if DomainRequest.status == statuses.STARTED %} | ||
{% with first_started_date=DomainRequest.get_first_status_started_date|date:"F j, Y" %} | ||
<p class="margin-top-1"> | ||
{% comment %} | ||
A newly created domain request will not have a value for last_status update. | ||
This is because the status never really updated. | ||
However, if this somehow goes back to started we can default to displaying that new date. | ||
{% endcomment %} | ||
<b class="review__step__name">Started on:</b> {{last_status_update|default:first_started_date}} | ||
</p> | ||
{% endwith %} | ||
{% elif DomainRequest.status == statuses.SUBMITTED %} | ||
<p class="margin-top-1 margin-bottom-1"> | ||
<b class="review__step__name">Submitted on:</b> {{last_submitted|default:first_submitted }} | ||
</p> | ||
<p class="margin-top-1"> | ||
<b class="review__step__name">Last updated on:</b> {{DomainRequest.updated_at|date:"F j, Y"}} | ||
</p> | ||
{% elif DomainRequest.status == statuses.ACTION_NEEDED %} | ||
<p class="margin-top-1 margin-bottom-1"> | ||
<b class="review__step__name">Submitted on:</b> {{last_submitted|default:first_submitted }} | ||
</p> | ||
<p class="margin-top-1"> | ||
<b class="review__step__name">Last updated on:</b> {{DomainRequest.updated_at|date:"F j, Y"}} | ||
</p> | ||
{% elif DomainRequest.status == statuses.REJECTED %} | ||
<p class="margin-top-1 margin-bottom-1"> | ||
<b class="review__step__name">Submitted on:</b> {{last_submitted|default:first_submitted }} | ||
</p> | ||
<p class="margin-top-1"> | ||
<b class="review__step__name">Rejected on:</b> {{last_status_update}} | ||
</p> | ||
{% elif DomainRequest.status == statuses.WITHDRAWN %} | ||
<p class="margin-top-1 margin-bottom-1"> | ||
<b class="review__step__name">Submitted on:</b> {{last_submitted|default:first_submitted }} | ||
</p> | ||
<p class="margin-top-1"> | ||
<b class="review__step__name">Withdrawn on:</b> {{last_status_update}} | ||
</p> | ||
{% else %} | ||
{% comment %} Shown for in_review, approved, ineligible {% endcomment %} | ||
<p class="margin-top-1"> | ||
<b class="review__step__name">Last updated on:</b> {{DomainRequest.updated_at|date:"F j, Y"}} | ||
</p> | ||
{% endif %} | ||
|
||
{% if DomainRequest.status != 'rejected' %} | ||
<p>{% include "includes/domain_request.html" %}</p> | ||
<p><a href="{% url 'domain-request-withdraw-confirmation' pk=DomainRequest.id %}" class="usa-button usa-button--outline withdraw_outline"> | ||
Withdraw request</a> | ||
</p> | ||
{% endif %} | ||
{% endwith %} | ||
</div> | ||
|
||
<div class="grid-col desktop:grid-offset-2 maxw-tablet"> | ||
|
@@ -141,8 +184,8 @@ <h2 class="text-primary-darker"> Summary of your domain request </h2> | |
{% if DomainRequest %} | ||
<h3 class="register-form-review-header">CISA Regional Representative</h3> | ||
<ul class="usa-list usa-list--unstyled margin-top-0"> | ||
{% if domain_request.cisa_representative_first_name %} | ||
{{domain_request.cisa_representative_first_name}} {{domain_request.cisa_representative_last_name}} | ||
{% if DomainRequest.cisa_representative_first_name %} | ||
{{ DomainRequest.get_formatted_cisa_rep_name }} | ||
{% else %} | ||
No | ||
{% endif %} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,9 @@ | |
import re | ||
from registrar.models.domain_request import DomainRequest | ||
from phonenumber_field.phonenumber import PhoneNumber | ||
from registrar.views.domain_request import DomainRequestWizard | ||
|
||
from registrar.models.utility.generic_helper import get_url_name | ||
|
||
register = template.Library() | ||
logger = logging.getLogger(__name__) | ||
|
@@ -174,3 +177,65 @@ def has_contact_info(user): | |
@register.filter | ||
def model_name_lowercase(instance): | ||
return instance.__class__.__name__.lower() | ||
|
||
|
||
@register.filter(name="is_domain_subpage") | ||
def is_domain_subpage(path): | ||
"""Checks if the given page is a subpage of domains. | ||
Takes a path name, like '/domains/'.""" | ||
# Since our pages aren't unified under a common path, we need this approach for now. | ||
url_names = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't think this is necessary to address in this PR, but I imagine this may get bigger as we introduce pages/subpages so should we consider a ticket to revisit this and refactor our paths in a way that's easier to recognize a page's more meta traits? @abroddrick There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree 100% with doing a refactor on our urls - and I share your concern about this list growing. Do you mind branching this off into a slack thread? I'm in favor of a refactor ticket on these as you mentioned! |
||
"domains", | ||
"no-portfolio-domains", | ||
"domain", | ||
"domain-users", | ||
"domain-dns", | ||
"domain-dns-nameservers", | ||
"domain-dns-dnssec", | ||
"domain-dns-dnssec-dsdata", | ||
"domain-your-contact-information", | ||
"domain-org-name-address", | ||
"domain-senior-official", | ||
"domain-security-email", | ||
"domain-users-add", | ||
"domain-request-delete", | ||
"domain-user-delete", | ||
"invitation-delete", | ||
] | ||
return get_url_name(path) in url_names | ||
|
||
|
||
@register.filter(name="is_domain_request_subpage") | ||
def is_domain_request_subpage(path): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dave-kennedy-ecs / @rachidatecs Just for your awareness, I had to add this logic to fix the headers. They weren't displaying correctly on many subpages. I've include this for portfolio and domains. The easier solution here would have been to check a url like |
||
"""Checks if the given page is a subpage of domain requests. | ||
Takes a path name, like '/requests/'.""" | ||
# Since our pages aren't unified under a common path, we need this approach for now. | ||
url_names = [ | ||
"domain-requests", | ||
"no-portfolio-requests", | ||
"domain-request-status", | ||
"domain-request-withdraw-confirmation", | ||
"domain-request-withdrawn", | ||
"domain-request-delete", | ||
] | ||
|
||
# The domain request wizard pages don't have a defined path, | ||
# so we need to check directly on it. | ||
wizard_paths = [ | ||
DomainRequestWizard.EDIT_URL_NAME, | ||
DomainRequestWizard.URL_NAMESPACE, | ||
DomainRequestWizard.NEW_URL_NAME, | ||
] | ||
return get_url_name(path) in url_names or any(wizard in path for wizard in wizard_paths) | ||
|
||
|
||
@register.filter(name="is_portfolio_subpage") | ||
def is_portfolio_subpage(path): | ||
"""Checks if the given page is a subpage of portfolio. | ||
Takes a path name, like '/organization/'.""" | ||
# Since our pages aren't unified under a common path, we need this approach for now. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the above comment that it may be worth revisiting how we organize our URL paths if we're adding more nested pages in the future |
||
url_names = [ | ||
"organization", | ||
"senior-official", | ||
] | ||
return get_url_name(path) in url_names |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, would we expect any cases where we'd want to get the date a request was first given a certain status aside from
started
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe for scripts or some info for django admin, but I'm not really concrete on it. I left this function around in case we need it, but honestly if you think it'd be more clear to just consolidate these I am totally OK with that too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also don't have a strong opinion so I'm happy to go with whatever data from audit logs you think would be useful in the future!