Skip to content

Commit

Permalink
feat: hide to_custom field when not needed
Browse files Browse the repository at this point in the history
  • Loading branch information
rpcross committed Nov 6, 2024
1 parent 3f2c745 commit 59c7b70
Show file tree
Hide file tree
Showing 10 changed files with 199 additions and 116 deletions.
16 changes: 9 additions & 7 deletions ietf/secr/announcement/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,15 @@ def get_to_choices():
# ---------------------------------------------

class AnnounceForm(forms.ModelForm):
nomcom = forms.ModelChoiceField(queryset=Group.objects.filter(acronym__startswith='nomcom',type='nomcom',state='active'),required=False)
nomcom = forms.ModelChoiceField(queryset=Group.objects.filter(acronym__startswith='nomcom', type='nomcom', state='active'), required=False)
to_custom = MultiEmailField(required=False)

class Meta:
model = Message
fields = ('nomcom', 'to','to_custom','frm','cc','bcc','reply_to','subject','body')
fields = ('nomcom', 'to', 'to_custom', 'frm', 'cc', 'bcc', 'reply_to', 'subject', 'body')
labels = {'frm': 'From'}
help_texts = {'to': 'Select name OR select Other... and enter email below',
'cc': 'Use comma separated lists for emails (Cc, Bcc, Reply To)'}

def __init__(self, *args, **kwargs):
if 'hidden' in kwargs:
Expand All @@ -91,19 +94,18 @@ def __init__(self, *args, **kwargs):
person = user.person
super(AnnounceForm, self).__init__(*args, **kwargs)
self.fields['to'].widget = forms.Select(choices=get_to_choices())
self.fields['to'].help_text = 'Select name OR select Other... and enter email below'
self.fields['cc'].help_text = 'Use comma separated lists for emails (Cc, Bcc, Reply To)'
self.fields['frm'].widget = forms.Select(choices=get_from_choices(user))
self.fields['frm'].label = 'From'
self.fields['reply_to'].required = True
# nomcom field is defined declaratively so label and help_text must be set here
self.fields['nomcom'].label = 'NomCom message:'
self.fields['nomcom'].help_text = 'If this is a NomCom announcement specifiy which NomCom group here'
nomcom_roles = person.role_set.filter(group__in=self.fields['nomcom'].queryset,name='chair')
secr_roles = person.role_set.filter(group__acronym='secretariat',name='secr')
if nomcom_roles:
self.initial['nomcom'] = nomcom_roles[0].group.pk
if not nomcom_roles and not secr_roles:
self.fields['nomcom'].widget = forms.HiddenInput()

if self.hidden:
for key in list(self.fields.keys()):
self.fields[key].widget = forms.HiddenInput()
Expand Down Expand Up @@ -134,4 +136,4 @@ def save(self, *args, **kwargs):
if nomcom:
message.related_groups.add(nomcom)

return message
return message
63 changes: 28 additions & 35 deletions ietf/secr/announcement/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
from ietf.message.models import AnnouncementFrom
from ietf.utils.mail import outbox, empty_outbox

SECR_USER='secretary'
WG_USER=''
AD_USER=''
SECR_USER = 'secretary'
WG_USER = ''
AD_USER = ''


class SecrAnnouncementTestCase(TestCase):
def setUp(self):
Expand All @@ -29,17 +30,17 @@ def setUp(self):
ietf = Group.objects.get(acronym='ietf')
iab = Group.objects.get(acronym='iab')
secretariat = Group.objects.get(acronym='secretariat')
AnnouncementFrom.objects.create(name=secr,group=secretariat,address='IETF Secretariat <[email protected]>')
AnnouncementFrom.objects.create(name=chair,group=ietf,address='IETF Chair <[email protected]>')
AnnouncementFrom.objects.create(name=chair,group=iab,address='IAB Chair <[email protected]>')
AnnouncementFrom.objects.create(name=secr, group=secretariat, address='IETF Secretariat <[email protected]>')
AnnouncementFrom.objects.create(name=chair, group=ietf, address='IETF Chair <[email protected]>')
AnnouncementFrom.objects.create(name=chair, group=iab, address='IAB Chair <[email protected]>')

def test_main(self):
"Main Test"
url = reverse('ietf.secr.announcement.views.main')
self.client.login(username="secretary", password="secretary+password")
r = self.client.get(url)
self.assertEqual(r.status_code, 200)

def test_main_announce_from(self):
url = reverse('ietf.secr.announcement.views.main')

Expand All @@ -48,44 +49,36 @@ def test_main_announce_from(self):
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
self.assertEqual(len(q('#id_frm option')),4)
self.assertEqual(len(q('#id_frm option')), 4)

# IAB Chair
self.client.login(username="iab-chair", password="iab-chair+password")
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
self.assertEqual(len(q('#id_frm option')),1)
self.assertEqual(len(q('#id_frm option')), 1)
self.assertTrue('<[email protected]>' in q('#id_frm option').val())

# IETF Chair
self.client.login(username="ietf-chair", password="ietf-chair+password")
r = self.client.get(url)
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
self.assertEqual(len(q('#id_frm option')),1)
self.assertEqual(len(q('#id_frm option')), 1)
self.assertTrue('<[email protected]>' in q('#id_frm option').val())


class UnauthorizedAnnouncementCase(TestCase):
def test_unauthorized(self):
"Unauthorized Test"
url = reverse('ietf.secr.announcement.views.main')
person = RoleFactory(name_id='chair',group__acronym='mars').person
self.client.login(username=person.user.username, password=person.user.username+"+password")
person = RoleFactory(name_id='chair', group__acronym='mars').person
self.client.login(username=person.user.username, password=person.user.username + "+password")
r = self.client.get(url)
self.assertEqual(r.status_code, 403)



class SubmitAnnouncementCase(TestCase):
def test_invalid_submit(self):
"Invalid Submit"
url = reverse('ietf.secr.announcement.views.main')
post_data = {'id_subject':''}
self.client.login(username="secretary", password="secretary+password")
r = self.client.post(url,post_data)
self.assertEqual(r.status_code, 200)
q = PyQuery(r.content)
self.assertTrue(len(q('form ul.errorlist')) > 0)

def test_valid_submit(self):
"Valid Submit"
nomcom_test_data()
Expand All @@ -94,20 +87,20 @@ def test_valid_submit(self):
confirm_url = reverse('ietf.secr.announcement.views.confirm')
nomcom = Group.objects.get(type='nomcom')
post_data = {'nomcom': nomcom.pk,
'to':'Other...',
'to_custom':'rcross@amsl.com',
'frm':'IETF Secretariat &lt;[email protected]&gt;',
'reply_to':'[email protected]',
'subject':'Test Subject',
'body':'This is a test.'}
'to': 'Other...',
'to_custom': 'phil@example.com',
'frm': 'IETF Secretariat &lt;[email protected]&gt;',
'reply_to': '[email protected]',
'subject': 'Test Subject',
'body': 'This is a test.'}
self.client.login(username="secretary", password="secretary+password")
response = self.client.post(url,post_data)
response = self.client.post(url, post_data)
self.assertContains(response, 'Confirm Announcement')
response = self.client.post(confirm_url,post_data,follow=True)
response = self.client.post(confirm_url, post_data,follow=True)
self.assertRedirects(response, url)
self.assertEqual(len(outbox),1)
self.assertEqual(outbox[0]['subject'],'Test Subject')
self.assertEqual(outbox[0]['to'],'<rcross@amsl.com>')
self.assertEqual(len(outbox), 1)
self.assertEqual(outbox[0]['subject'], 'Test Subject')
self.assertEqual(outbox[0]['to'], '<phil@example.com>')
message = Message.objects.filter(by__user__username='secretary').last()
self.assertEqual(message.subject,'Test Subject')
self.assertEqual(message.subject, 'Test Subject')
self.assertTrue(nomcom in message.related_groups.all())
16 changes: 6 additions & 10 deletions ietf/secr/announcement/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def main(request):
if not check_access(request.user):
permission_denied(request, 'Restricted to: Secretariat, IAD, or chair of IETF, IAB, RSOC, RSE, IAOC, ISOC, NomCom.')

form = AnnounceForm(request.POST or None,user=request.user)
form = AnnounceForm(request.POST or None, user=request.user)

if form.is_valid():
# recast as hidden form for next page of process
Expand All @@ -71,7 +71,8 @@ def main(request):
'form': form},
)

return render(request, 'announcement/index.html', { 'form': form} )
return render(request, 'announcement/index.html', {'form': form})


@login_required
@check_for_cancel('../')
Expand All @@ -83,21 +84,16 @@ def confirm(request):
if request.method == 'POST':
form = AnnounceForm(request.POST, user=request.user)
if request.method == 'POST':
message = form.save(user=request.user,commit=True)
extra = {'Reply-To': message.get('reply_to') }
message = form.save(user=request.user, commit=True)
extra = {'Reply-To': message.get('reply_to')}
send_mail_text(None,
message.to,
message.frm,
message.subject,
message.body,
cc=message.cc,
bcc=message.bcc,
extra=extra,
)
extra=extra)

messages.success(request, 'The announcement was sent.')
return redirect('ietf.secr.announcement.views.main')




4 changes: 2 additions & 2 deletions ietf/secr/templates/announcement/confirm.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{# Copyright The IETF Trust 2007, All Rights Reserved #}
{# Copyright The IETF Trust 2024, All Rights Reserved #}
{% extends "base.html" %}
{% load static %}
{% load ietf_filters %}
Expand Down Expand Up @@ -26,7 +26,7 @@ <h2>Confirm Announcement</h2>

{% bootstrap_form form %}

<div class="d-flex justify-content-evenly">
<div class="d-flex justify-content-around">
<button class="btn btn-primary" type="submit" name="submit" value="Send">Send</button>
<button class="btn btn-secondary" type="button" onclick="history.go(-1);return false">Back</button>
<button class="btn btn-danger" type="submit" name="submit" value="Cancel">Cancel</button>
Expand Down
43 changes: 18 additions & 25 deletions ietf/secr/templates/announcement/index.html
Original file line number Diff line number Diff line change
@@ -1,38 +1,31 @@
{# Copyright The IETF Trust 2007, All Rights Reserved #}
{# Copyright The IETF Trust 2024, All Rights Reserved #}
{% extends "base.html" %}
{% load static %}
{% load ietf_filters %}
{% load django_bootstrap5 %}
{% block title %}Announcement{% endblock %}
{% block content %}
<h1>Announcement</h1>
<h1 class="announcement">Announcement</h1>
{% if form.non_field_errors %}<div class="my-3 alert alert-danger">{{ form.non_field_errors }}</div>{% endif %}

<form method="post">
<form id="announcement-form" method="post">
{% csrf_token %}
{% bootstrap_form form %}
{% bootstrap_field form.nomcom layout='horizontal' %}
{% bootstrap_field form.to layout='horizontal' %}
{% bootstrap_field form.to_custom layout='horizontal' %}
{% bootstrap_field form.frm layout='horizontal' %}
{% bootstrap_field form.cc layout='horizontal' %}
{% bootstrap_field form.bcc layout='horizontal' %}
{% bootstrap_field form.reply_to layout='horizontal' %}
{% bootstrap_field form.subject layout='horizontal' %}
{% bootstrap_field form.body layout='horizontal' %}

<button type="submit" class="btn btn-primary">Continue</button>
<a class="btn btn-secondary float-end"
href="{% url 'ietf.group.views.streams' %}{{ group.acronym }}">Back</a>
href="{% url 'ietf.secr' %}">Cancel</a>
</form>

<form method="post">{% csrf_token %}
<table class="new-style full-width amstable" id="announce-table">
<tbody>
{% if form.non_field_errors %}{{ form.non_field_errors }}{% endif %}
{% for field in form.visible_fields %}
<tr>
<th scope="row">{{ field.label_tag }}{% if field.field.required %}<span class="required"> *</span>{% endif %}</th>
<td>{{ field.errors }}{{ field }}{% if field.help_text %}<br>{{ field.help_text }}{% endif %}</td>
</tr>
{% endfor %}
</tbody>
</table>
<div class="button-group">
<ul id="announcement-button-list">
<li><button type="submit" name="submit" value="Continue">Continue</button></li>
<li><button type="button" onclick="window.location='../'" value="Cancel">Cancel</button></li>
</ul>
</div> <!-- button-group -->

</form>
{% endblock %}
{% block js %}
<script src="{% static 'ietf/js/announcement.js' %}"></script>
{% endblock %}
36 changes: 0 additions & 36 deletions ietf/secr/templates/announcement/main.html

This file was deleted.

2 changes: 1 addition & 1 deletion ietf/secr/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from django.views.generic import TemplateView

urlpatterns = [
re_path(r'^$', TemplateView.as_view(template_name='index.html')),
re_path(r'^$', TemplateView.as_view(template_name='index.html'), name='ietf.secr'),
re_path(r'^announcement/', include('ietf.secr.announcement.urls')),
re_path(r'^meetings/', include('ietf.secr.meetings.urls')),
re_path(r'^rolodex/', include('ietf.secr.rolodex.urls')),
Expand Down
57 changes: 57 additions & 0 deletions ietf/static/js/announcement.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
const announcementApp = (function() {
'use strict';
return {
// functions for Announcement
checkToField: function() {
document.documentElement.scrollTop = 0; // For most browsers
const toField = document.getElementById('id_to');
const toCustomInput = document.getElementById('id_to_custom');
const toCustomDiv = toCustomInput.closest('div.row');

if (toField.value === 'Other...') {
toCustomDiv.style.display = 'flex'; // Show the custom field
} else {
toCustomDiv.style.display = 'none'; // Hide the custom field
toCustomInput.value = ''; // Optionally clear the input value if hidden
}
}
};
})();

// Extra care is required to ensure the back button
// works properly for the optional to_custom field.
// Take the case when a user selects "Other..." for
// "To" field. The "To custom" field appears and they
// enter a new address there.
// In Chrome, when the form is submitted and then the user
// uses the back button (or browser back), the page loads
// from bfcache then the javascript DOMContentLoaded event
// handler is run, hiding the empty to_custom field, THEN the
// browser autofills the form fields. Because to_submit
// is now hidden it does not get a value. This is a very
// bad experience for the user because the to_custom field
// was unexpectedly cleared and hidden. If they notice this
// they would need to know to first select another "To"
// option, then select "Other..." again just to get the
// to_custom field visible so they can re-enter the custom
// address.
// The solution is to use setTimeout to run checkToField
// after a short delay, giving the browser time to autofill
// the form fields before it checks to see if the to_custom
// field is empty and hides it.

document.addEventListener('DOMContentLoaded', function() {
// Run the visibility check after allowing cache to populate values
setTimeout(announcementApp.checkToField, 300);

const toField = document.getElementById('id_to');
toField.addEventListener('change', announcementApp.checkToField);
});

// Handle back/forward navigation with pageshow
window.addEventListener('pageshow', function(event) {
if (event.persisted) {
// Then apply visibility logic after cache restoration
setTimeout(announcementApp.checkToField, 300);
}
});
1 change: 1 addition & 0 deletions playwright/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ node_modules/
/test-results/
/playwright-report/
/playwright/.cache/
auth.json
Loading

0 comments on commit 59c7b70

Please sign in to comment.