From b84470705acbc05329c27a0ec545bd1dc6f3f849 Mon Sep 17 00:00:00 2001 From: Ryan Cross Date: Sun, 3 Nov 2024 15:37:05 +0000 Subject: [PATCH 1/6] refactor: update secretariat dashboard style --- ietf/secr/templates/index.html | 33 ++++++++++++++++ ietf/secr/templates/main.html | 69 ---------------------------------- ietf/secr/urls.py | 2 +- 3 files changed, 34 insertions(+), 70 deletions(-) create mode 100644 ietf/secr/templates/index.html delete mode 100644 ietf/secr/templates/main.html diff --git a/ietf/secr/templates/index.html b/ietf/secr/templates/index.html new file mode 100644 index 00000000000..05fa3db41f0 --- /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 42d6e8f6a13..00000000000 --- 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 0ce14a449aa..aa264f9ac3e 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')), 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')), From 3f2c745b044a58da17978da2c04ca3737f44d186 Mon Sep 17 00:00:00 2001 From: Ryan Cross Date: Sun, 3 Nov 2024 16:19:33 +0000 Subject: [PATCH 2/6] refactor: change announcement styling --- ietf/secr/announcement/views.py | 2 +- ietf/secr/templates/announcement/confirm.html | 33 +++++++--------- ietf/secr/templates/announcement/index.html | 38 +++++++++++++++++++ 3 files changed, 52 insertions(+), 21 deletions(-) create mode 100644 ietf/secr/templates/announcement/index.html diff --git a/ietf/secr/announcement/views.py b/ietf/secr/announcement/views.py index 42de089c59c..078849cfb36 100644 --- a/ietf/secr/announcement/views.py +++ b/ietf/secr/announcement/views.py @@ -71,7 +71,7 @@ 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('../') diff --git a/ietf/secr/templates/announcement/confirm.html b/ietf/secr/templates/announcement/confirm.html index ddf2a6de6ed..44ff4dcfb68 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 2007, 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 00000000000..e237dba926e --- /dev/null +++ b/ietf/secr/templates/announcement/index.html @@ -0,0 +1,38 @@ +{# Copyright The IETF Trust 2007, All Rights Reserved #} +{% extends "base.html" %} +{% load static %} +{% load ietf_filters %} +{% load django_bootstrap5 %} +{% block title %}Announcement{% endblock %} +{% block content %} +

Announcement

+ +
+ {% csrf_token %} + {% bootstrap_form form %} + + Back +
+ +
{% 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 %} \ No newline at end of file From 59c7b7060f1a5804f3ee7a115af683960941786b Mon Sep 17 00:00:00 2001 From: Ryan Cross Date: Wed, 6 Nov 2024 15:51:39 +0000 Subject: [PATCH 3/6] feat: hide to_custom field when not needed --- ietf/secr/announcement/forms.py | 16 ++-- ietf/secr/announcement/tests.py | 63 +++++++-------- ietf/secr/announcement/views.py | 16 ++-- ietf/secr/templates/announcement/confirm.html | 4 +- ietf/secr/templates/announcement/index.html | 43 +++++------ ietf/secr/templates/announcement/main.html | 36 --------- ietf/secr/urls.py | 2 +- ietf/static/js/announcement.js | 57 ++++++++++++++ playwright/.gitignore | 1 + playwright/tests/secr/announcement.spec.js | 77 +++++++++++++++++++ 10 files changed, 199 insertions(+), 116 deletions(-) delete mode 100644 ietf/secr/templates/announcement/main.html create mode 100644 ietf/static/js/announcement.js create mode 100644 playwright/tests/secr/announcement.spec.js diff --git a/ietf/secr/announcement/forms.py b/ietf/secr/announcement/forms.py index 3fe58bdaaab..96d386bce54 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 c147c301b61..66204a47cc6 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 078849cfb36..0d1642539e6 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/index.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 44ff4dcfb68..0e1f72c54b2 100644 --- a/ietf/secr/templates/announcement/confirm.html +++ b/ietf/secr/templates/announcement/confirm.html @@ -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 %} @@ -26,7 +26,7 @@

Confirm Announcement

{% bootstrap_form form %} -
+
diff --git a/ietf/secr/templates/announcement/index.html b/ietf/secr/templates/announcement/index.html index e237dba926e..ad7226e3bc1 100644 --- a/ietf/secr/templates/announcement/index.html +++ b/ietf/secr/templates/announcement/index.html @@ -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 %} -

Announcement

+

Announcement

+ {% if form.non_field_errors %}
{{ form.non_field_errors }}
{% endif %} -
+ {% 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' %} + Back + href="{% url 'ietf.secr' %}">Cancel
-
{% 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 %} +{% 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 c88b4a2406c..00000000000 --- 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/urls.py b/ietf/secr/urls.py index aa264f9ac3e..4a3e5b0363f 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='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')), diff --git a/ietf/static/js/announcement.js b/ietf/static/js/announcement.js new file mode 100644 index 00000000000..95465120fa8 --- /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 75e854d8dcf..f38d036a792 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 00000000000..4dbbc25a813 --- /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 From 1914d683b936ae1daf5aae27658b671343ff37d9 Mon Sep 17 00:00:00 2001 From: Ryan Cross Date: Fri, 18 Apr 2025 16:20:02 -0700 Subject: [PATCH 4/6] fix: apply Black to python files --- ietf/secr/announcement/forms.py | 133 +++++++++++++++++++++----------- ietf/secr/announcement/tests.py | 88 ++++++++++++--------- ietf/secr/announcement/urls.py | 5 +- ietf/secr/announcement/views.py | 85 +++++++++++--------- 4 files changed, 186 insertions(+), 125 deletions(-) diff --git a/ietf/secr/announcement/forms.py b/ietf/secr/announcement/forms.py index 96d386bce54..820ef79a093 100644 --- a/ietf/secr/announcement/forms.py +++ b/ietf/secr/announcement/forms.py @@ -14,30 +14,41 @@ # Globals # --------------------------------------------- -TO_LIST = ('IETF Announcement List ', - 'I-D Announcement List ', - 'RFP Announcement List ', - 'The IESG ', - 'Working Group Chairs ', - 'BOF Chairs ', - 'Other...') +TO_LIST = ( + "IETF Announcement List ", + "I-D Announcement List ", + "RFP Announcement List ", + "The IESG ", + "Working Group Chairs ", + "BOF Chairs ", + "Other...", +) # --------------------------------------------- # Helper Functions # --------------------------------------------- + def get_from_choices(user): - ''' + """ This function returns a choices tuple containing all the Announced From choices. Including leadership chairs and other entities. - ''' + """ addresses = [] - if has_role(user,'Secretariat'): - addresses = AnnouncementFrom.objects.values_list('address', flat=True).order_by('address').distinct() + if has_role(user, "Secretariat"): + addresses = ( + AnnouncementFrom.objects.values_list("address", flat=True) + .order_by("address") + .distinct() + ) else: for role in user.person.role_set.all(): - addresses.extend(AnnouncementFrom.objects.filter(name=role.name, group=role.group).values_list('address', flat=True).order_by('address')) + addresses.extend( + AnnouncementFrom.objects.filter(name=role.name, group=role.group) + .values_list("address", flat=True) + .order_by("address") + ) nomcom_choices = get_nomcom_choices(user) if nomcom_choices: @@ -45,66 +56,90 @@ def get_from_choices(user): choices = list(zip(addresses, addresses)) if len(choices) > 1: - choices.insert(0, ('', '(Choose an option)')) + choices.insert(0, ("", "(Choose an option)")) return choices def get_nomcom_choices(user): - ''' + """ Returns the list of nomcom email addresses for given user - ''' - nomcoms = Role.objects.filter(name="chair", - group__acronym__startswith="nomcom", - group__state="active", - group__type="nomcom", - person=user.person) + """ + nomcoms = Role.objects.filter( + name="chair", + group__acronym__startswith="nomcom", + group__state="active", + group__type="nomcom", + person=user.person, + ) addresses = [] for nomcom in nomcoms: year = nomcom.group.acronym[-4:] - addresses.append('NomCom Chair %s ' % (year,year)) + addresses.append("NomCom Chair %s " % (year, year)) return addresses - + def get_to_choices(): - return list(zip(TO_LIST,TO_LIST)) + return list(zip(TO_LIST, TO_LIST)) # --------------------------------------------- # Forms # --------------------------------------------- + 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') - 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)'} + 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: - self.hidden = kwargs.pop('hidden') + if "hidden" in kwargs: + self.hidden = kwargs.pop("hidden") else: self.hidden = False - user = kwargs.pop('user') + user = kwargs.pop("user") person = user.person super(AnnounceForm, self).__init__(*args, **kwargs) - self.fields['to'].widget = forms.Select(choices=get_to_choices()) - self.fields['frm'].widget = forms.Select(choices=get_from_choices(user)) - self.fields['reply_to'].required = True + self.fields["to"].widget = forms.Select(choices=get_to_choices()) + self.fields["frm"].widget = forms.Select(choices=get_from_choices(user)) + 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') + 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 + self.initial["nomcom"] = nomcom_roles[0].group.pk if not nomcom_roles and not secr_roles: - self.fields['nomcom'].widget = forms.HiddenInput() + self.fields["nomcom"].widget = forms.HiddenInput() if self.hidden: for key in list(self.fields.keys()): @@ -115,24 +150,28 @@ def clean(self): data = self.cleaned_data if self.errors: return self.cleaned_data - if data['to'] == 'Other...' and not data['to_custom']: + if data["to"] == "Other..." and not data["to_custom"]: raise forms.ValidationError('You must enter a "To" email address') - for k in ['to', 'frm', 'cc',]: + for k in [ + "to", + "frm", + "cc", + ]: data[k] = unescape(data[k]) return data def save(self, *args, **kwargs): - user = kwargs.pop('user') + user = kwargs.pop("user") message = super(AnnounceForm, self).save(commit=False) message.by = user.person - if self.cleaned_data['to'] == 'Other...': - message.to = self.cleaned_data['to_custom'] - if kwargs['commit']: + if self.cleaned_data["to"] == "Other...": + message.to = self.cleaned_data["to_custom"] + if kwargs["commit"]: message.save() # handle nomcom message - nomcom = self.cleaned_data.get('nomcom',False) + nomcom = self.cleaned_data.get("nomcom", False) if nomcom: message.related_groups.add(nomcom) diff --git a/ietf/secr/announcement/tests.py b/ietf/secr/announcement/tests.py index 66204a47cc6..f08e824397e 100644 --- a/ietf/secr/announcement/tests.py +++ b/ietf/secr/announcement/tests.py @@ -6,7 +6,7 @@ from django.urls import reverse -import debug # pyflakes:ignore +import debug # pyflakes:ignore from ietf.utils.test_utils import TestCase from ietf.group.factories import RoleFactory @@ -17,63 +17,73 @@ 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): super().setUp() - chair = RoleName.objects.get(slug='chair') - secr = RoleName.objects.get(slug='secr') - 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 ') + chair = RoleName.objects.get(slug="chair") + secr = RoleName.objects.get(slug="secr") + 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 " + ) def test_main(self): "Main Test" - url = reverse('ietf.secr.announcement.views.main') + 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') + url = reverse("ietf.secr.announcement.views.main") # Secretariat self.client.login(username="secretary", password="secretary+password") 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.assertTrue('' in q('#id_frm option').val()) + self.assertEqual(len(q("#id_frm option")), 1) + self.assertTrue("" 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.assertTrue('' in q('#id_frm option').val()) + 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") + 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" + ) r = self.client.get(url) self.assertEqual(r.status_code, 403) @@ -83,24 +93,26 @@ def test_valid_submit(self): "Valid Submit" nomcom_test_data() empty_outbox() - url = reverse('ietf.secr.announcement.views.main') - confirm_url = reverse('ietf.secr.announcement.views.confirm') - nomcom = Group.objects.get(type='nomcom') - post_data = {'nomcom': nomcom.pk, - '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.'} + url = reverse("ietf.secr.announcement.views.main") + confirm_url = reverse("ietf.secr.announcement.views.confirm") + nomcom = Group.objects.get(type="nomcom") + post_data = { + "nomcom": nomcom.pk, + "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) - self.assertContains(response, 'Confirm Announcement') - response = self.client.post(confirm_url, post_data,follow=True) + self.assertContains(response, "Confirm Announcement") + 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'], '') - message = Message.objects.filter(by__user__username='secretary').last() - self.assertEqual(message.subject, 'Test Subject') + 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.assertTrue(nomcom in message.related_groups.all()) diff --git a/ietf/secr/announcement/urls.py b/ietf/secr/announcement/urls.py index 3c3c05a09c4..dc534f64aeb 100644 --- a/ietf/secr/announcement/urls.py +++ b/ietf/secr/announcement/urls.py @@ -1,8 +1,7 @@ - from ietf.secr.announcement import views from ietf.utils.urls import url urlpatterns = [ - url(r'^$', views.main), - url(r'^confirm/$', views.confirm), + url(r"^$", views.main), + url(r"^confirm/$", views.confirm), ] diff --git a/ietf/secr/announcement/views.py b/ietf/secr/announcement/views.py index 0d1642539e6..5617ae9e6fe 100644 --- a/ietf/secr/announcement/views.py +++ b/ietf/secr/announcement/views.py @@ -18,82 +18,93 @@ # Helper Functions # ------------------------------------------------- def check_access(user): - ''' + """ This function takes a Django User object and returns true if the user has access to the Announcement app. - ''' + """ if hasattr(user, "person"): person = user.person if has_role(user, "Secretariat"): return True - + for role in person.role_set.all(): - if AnnouncementFrom.objects.filter(name=role.name,group=role.group): + if AnnouncementFrom.objects.filter(name=role.name, group=role.group): return True - if Role.objects.filter(name="chair", - group__acronym__startswith="nomcom", - group__state="active", - group__type="nomcom", - person=person): + if Role.objects.filter( + name="chair", + group__acronym__startswith="nomcom", + group__state="active", + group__type="nomcom", + person=person, + ): return True return False + # -------------------------------------------------- # STANDARD VIEW FUNCTIONS # -------------------------------------------------- # this seems to cause some kind of circular problem # @check_for_cancel(reverse('home')) @login_required -@check_for_cancel('../') +@check_for_cancel("../") def main(request): - ''' + """ Main view for Announcement tool. Authrozied users can fill out email details: header, body, etc and send. - ''' + """ if not check_access(request.user): - permission_denied(request, 'Restricted to: Secretariat, IAD, or chair of IETF, IAB, RSOC, RSE, IAOC, ISOC, NomCom.') + 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) if form.is_valid(): # recast as hidden form for next page of process form = AnnounceForm(request.POST, user=request.user, hidden=True) - if form.data['to'] == 'Other...': - to = form.data['to_custom'] + if form.data["to"] == "Other...": + to = form.data["to_custom"] else: - to = form.data['to'] + to = form.data["to"] - return render(request, 'announcement/confirm.html', { - 'message': form.data, - 'to': to, - 'form': form}, + return render( + request, + "announcement/confirm.html", + {"message": form.data, "to": to, "form": form}, ) - return render(request, 'announcement/index.html', {'form': form}) + return render(request, "announcement/index.html", {"form": form}) @login_required -@check_for_cancel('../') +@check_for_cancel("../") def confirm(request): if not check_access(request.user): - permission_denied(request, 'Restricted to: Secretariat, IAD, or chair of IETF, IAB, RSOC, RSE, IAOC, ISOC, NomCom.') + permission_denied( + request, + "Restricted to: Secretariat, IAD, or chair of IETF, IAB, RSOC, RSE, IAOC, ISOC, NomCom.", + ) - if request.method == 'POST': + if request.method == "POST": form = AnnounceForm(request.POST, user=request.user) - if request.method == 'POST': + if request.method == "POST": 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) - - messages.success(request, 'The announcement was sent.') - return redirect('ietf.secr.announcement.views.main') + 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, + ) + + messages.success(request, "The announcement was sent.") + return redirect("ietf.secr.announcement.views.main") From 02ebe425bb98508e1fde87022fe4fe54849bb1a0 Mon Sep 17 00:00:00 2001 From: Ryan Cross Date: Sun, 20 Apr 2025 21:03:10 -0700 Subject: [PATCH 5/6] fix: add announcement.js to package.json --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index afcabd13edb..59b26229101 100644 --- a/package.json +++ b/package.json @@ -113,6 +113,7 @@ "ietf/static/images/irtf-logo.svg", "ietf/static/js/agenda_filter.js", "ietf/static/js/agenda_materials.js", + "ietf/static/js/announcement.js", "ietf/static/js/complete-review.js", "ietf/static/js/create_timeslot.js", "ietf/static/js/create_timeslot.js", From a3bd596c43ce45b5aa0f898c33c63426eeca6581 Mon Sep 17 00:00:00 2001 From: Ryan Cross Date: Wed, 23 Apr 2025 08:24:56 -0700 Subject: [PATCH 6/6] fix: move announcement playwright test to tests-legacy --- playwright/{tests => tests-legacy}/secr/announcement.spec.js | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename playwright/{tests => tests-legacy}/secr/announcement.spec.js (100%) diff --git a/playwright/tests/secr/announcement.spec.js b/playwright/tests-legacy/secr/announcement.spec.js similarity index 100% rename from playwright/tests/secr/announcement.spec.js rename to playwright/tests-legacy/secr/announcement.spec.js