diff --git a/ietf/secr/announcement/forms.py b/ietf/secr/announcement/forms.py index 3fe58bdaaa..96d386bce5 100644 --- a/ietf/secr/announcement/forms.py +++ b/ietf/secr/announcement/forms.py @@ -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: @@ -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() @@ -134,4 +136,4 @@ def save(self, *args, **kwargs): if nomcom: message.related_groups.add(nomcom) - return message \ No newline at end of file + return message diff --git a/ietf/secr/announcement/tests.py b/ietf/secr/announcement/tests.py index c147c301b6..66204a47cc 100644 --- a/ietf/secr/announcement/tests.py +++ b/ietf/secr/announcement/tests.py @@ -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): @@ -29,9 +30,9 @@ 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 ') - AnnouncementFrom.objects.create(name=chair,group=ietf,address='IETF Chair ') - AnnouncementFrom.objects.create(name=chair,group=iab,address='IAB Chair ') + AnnouncementFrom.objects.create(name=secr, group=secretariat, address='IETF Secretariat ') + AnnouncementFrom.objects.create(name=chair, group=ietf, address='IETF Chair ') + AnnouncementFrom.objects.create(name=chair, group=iab, address='IAB Chair ') def test_main(self): "Main Test" @@ -39,7 +40,7 @@ def test_main(self): 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') @@ -48,14 +49,14 @@ 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('' in q('#id_frm option').val()) # IETF Chair @@ -63,29 +64,21 @@ 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')),1) + self.assertEqual(len(q('#id_frm option')), 1) self.assertTrue('' 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() @@ -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 <ietf-secretariat@ietf.org>', - 'reply_to':'secretariat@ietf.org', - 'subject':'Test Subject', - 'body':'This is a test.'} + 'to': 'Other...', + 'to_custom': 'phil@example.com', + 'frm': 'IETF Secretariat <ietf-secretariat@ietf.org>', + 'reply_to': 'secretariat@ietf.org', + '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'],'') + self.assertEqual(len(outbox), 1) + self.assertEqual(outbox[0]['subject'], 'Test Subject') + self.assertEqual(outbox[0]['to'], '') 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()) diff --git a/ietf/secr/announcement/views.py b/ietf/secr/announcement/views.py index 42de089c59..0d1642539e 100644 --- a/ietf/secr/announcement/views.py +++ b/ietf/secr/announcement/views.py @@ -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 @@ -71,7 +71,8 @@ def main(request): 'form': form}, ) - return render(request, 'announcement/main.html', { 'form': form} ) + return render(request, 'announcement/index.html', {'form': form}) + @login_required @check_for_cancel('../') @@ -83,8 +84,8 @@ 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, @@ -92,12 +93,7 @@ def confirm(request): 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') - - - - diff --git a/ietf/secr/templates/announcement/confirm.html b/ietf/secr/templates/announcement/confirm.html index ddf2a6de6e..0e1f72c54b 100644 --- a/ietf/secr/templates/announcement/confirm.html +++ b/ietf/secr/templates/announcement/confirm.html @@ -1,22 +1,17 @@ -{% extends "base_site.html" %} +{# Copyright The IETF Trust 2024, All Rights Reserved #} +{% extends "base.html" %} {% load static %} - +{% load ietf_filters %} +{% load django_bootstrap5 %} {% block title %}Announcement{% endblock %} - -{% block extrahead %}{{ block.super }} - -{% endblock %} - -{% block breadcrumbs %}{{ block.super }} - » Announcement -{% endblock %} - {% block content %} +

Announcement

+

Confirm Announcement

-
{% csrf_token %} + {% csrf_token %} 
 To: {{ to }}
@@ -29,15 +24,13 @@ 
Confirm Announcement

 {{ message.body }}
- {{ form }} -
-
    -
    • -
    • -
    • -
-
+ {% bootstrap_form form %} +
+ + + +
diff --git a/ietf/secr/templates/announcement/index.html b/ietf/secr/templates/announcement/index.html new file mode 100644 index 0000000000..ad7226e3bc --- /dev/null +++ b/ietf/secr/templates/announcement/index.html @@ -0,0 +1,31 @@ +{# 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 %} +

Announcement

+ {% if form.non_field_errors %}
{{ form.non_field_errors }}
{% endif %} + +
+ {% csrf_token %} + {% 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' %} + + + Cancel +
+ +{% endblock %} +{% block js %} + +{% endblock %} \ No newline at end of file diff --git a/ietf/secr/templates/announcement/main.html b/ietf/secr/templates/announcement/main.html deleted file mode 100644 index c88b4a2406..0000000000 --- a/ietf/secr/templates/announcement/main.html +++ /dev/null @@ -1,36 +0,0 @@ -{% extends "base_site.html" %} - -{% block title %}Announcement{% endblock %} - -{% block breadcrumbs %}{{ block.super }} - » Announcement -{% endblock %} - -{% block content %} - -
-

Announcement

- -
{% csrf_token %} - - - {% if form.non_field_errors %}{{ form.non_field_errors }}{% endif %} - {% for field in form.visible_fields %} - - - - - {% endfor %} - -
{{ field.label_tag }}{% if field.field.required %} *{% endif %}{{ field.errors }}{{ field }}{% if field.help_text %}
{{ field.help_text }}{% endif %}
-
-
    -
    • -
    • -
-
- -
-
- -{% endblock %} diff --git a/ietf/secr/templates/index.html b/ietf/secr/templates/index.html new file mode 100644 index 0000000000..05fa3db41f --- /dev/null +++ b/ietf/secr/templates/index.html @@ -0,0 +1,33 @@ +{# Copyright The IETF Trust 2007, All Rights Reserved #} +{% extends "base.html" %} +{% load static %} +{% load ietf_filters %} +{% block title %}Secretariat Dashboard{% endblock %} +{% block content %} +

Secretariat Dashboard

+
+ {% if user|has_role:"Secretariat" %} +

IESG

+ + +

IDs and WGs Process

+ + +

Meetings and Proceedings

+ + {% else %} + + {% endif %} +
+{% endblock %} \ No newline at end of file diff --git a/ietf/secr/templates/main.html b/ietf/secr/templates/main.html deleted file mode 100644 index 42d6e8f6a1..0000000000 --- a/ietf/secr/templates/main.html +++ /dev/null @@ -1,69 +0,0 @@ -{% extends "base_site.html" %} -{% load ietf_filters %} - -{% block content %} -
- - {% if user|has_role:"Secretariat" %} - - - - - - - - - - - - -
-

IESG

- - 		-

IDs and WGs Process

- -
-

Meetings and Proceedings

- -
- - {% else %} - - - - - - - - - - - - -
-

Section 1

- - 		-

Section 2

-
-

Section 3

- 		-

Section 4

-
- - {% endif %} - -
-{% endblock %} \ No newline at end of file diff --git a/ietf/secr/urls.py b/ietf/secr/urls.py index 0ce14a449a..4a3e5b0363 100644 --- a/ietf/secr/urls.py +++ b/ietf/secr/urls.py @@ -2,7 +2,7 @@ from django.views.generic import TemplateView urlpatterns = [ - re_path(r'^$', TemplateView.as_view(template_name='main.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')), diff --git a/ietf/static/js/announcement.js b/ietf/static/js/announcement.js new file mode 100644 index 0000000000..95465120fa --- /dev/null +++ b/ietf/static/js/announcement.js @@ -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); + } +}); \ No newline at end of file diff --git a/playwright/.gitignore b/playwright/.gitignore index 75e854d8dc..f38d036a79 100644 --- a/playwright/.gitignore +++ b/playwright/.gitignore @@ -2,3 +2,4 @@ node_modules/ /test-results/ /playwright-report/ /playwright/.cache/ +auth.json \ No newline at end of file diff --git a/playwright/tests/secr/announcement.spec.js b/playwright/tests/secr/announcement.spec.js new file mode 100644 index 0000000000..4dbbc25a81 --- /dev/null +++ b/playwright/tests/secr/announcement.spec.js @@ -0,0 +1,77 @@ +const { test, expect } = require('@playwright/test') +const viewports = require('../../helpers/viewports') +const { setTimeout } = require('timers/promises') + +// ==================================================================== +// ANNOUNCEMENT | DESKTOP viewport +// ==================================================================== + +test.describe('desktop', () => { + + test.beforeAll(async ({ browser }) => { + const context = await browser.newContext(); + const page = await context.newPage(); + + await page.goto('/accounts/login/'); + + await page.fill('input#id_username', 'glen'); + await page.fill('input#id_password', 'password'); + + await page.click('button[type="submit"]'); + await page.waitForURL('/accounts/profile/'); + + await context.storageState({ path: 'auth.json' }); + + await context.close(); + }); + + test.beforeEach(async ({ browser }) => { + // Reuse the authentication state in each test + const context = await browser.newContext({ storageState: 'auth.json' }); + const page = await context.newPage(); + await page.setViewportSize({ + width: viewports.desktop[0], + height: viewports.desktop[1] + }) + await page.goto(`/secr/announcement/`); + await page.locator('h1:text("Announcement")').waitFor({ state: 'visible' }) + await setTimeout(500) + // Attach the page to the test context + test.info().page = page; + }) + + test('show to custom', async () => { + const page = test.info().page; + + // to_custom should initially be hidden + const element = page.locator('#id_to_custom'); + await expect(element).toBeHidden(); + await page.selectOption('select#id_to', 'Other...'); + await expect(element).toBeVisible(); + }) + + test('back button', async () => { + const page = test.info().page; + + const element = page.locator('#id_to_custom'); + await page.selectOption('select#id_to', 'Other...'); + await expect(element).toBeVisible(); + await page.fill('input#id_to_custom', 'custom@example.com'); + await page.selectOption('select#id_frm', 'IETF Chair '); + await page.fill('input#id_reply_to', 'greg@example.com'); + await page.fill('input#id_subject', 'About Stuff'); + await page.fill('textarea#id_body', 'This is the stuff'); + + await page.click('text="Continue"'); + const h2Locator = page.locator('h2:text("Confirm Announcement")'); + await h2Locator.waitFor({ state: 'visible' }); + + // click back button and check to_custom + await page.click('text="Back"'); + const subjectLocator = page.locator('input#id_subject'); + await subjectLocator.waitFor({ state: 'visible' }); + await expect(element).toBeVisible(); + await expect(element).toHaveValue('custom@example.com'); + }) + +}) \ No newline at end of file