From a238e7bbd42074c285114b1a9388ece91db03ee9 Mon Sep 17 00:00:00 2001 From: Steven Eardley Date: Thu, 3 Oct 2024 13:46:35 +0100 Subject: [PATCH 1/3] update tests to account for removed route in editor.py --- ...plication_forms.py => _disabled_test_application_forms.py} | 4 ++++ .../events/consumers/journal_editor_group_assigned_notify.py | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) rename doajtest/unit/{test_application_forms.py => _disabled_test_application_forms.py} (95%) diff --git a/doajtest/unit/test_application_forms.py b/doajtest/unit/_disabled_test_application_forms.py similarity index 95% rename from doajtest/unit/test_application_forms.py rename to doajtest/unit/_disabled_test_application_forms.py index 94e17ab761..41a46cfb5b 100644 --- a/doajtest/unit/test_application_forms.py +++ b/doajtest/unit/_disabled_test_application_forms.py @@ -1,3 +1,7 @@ +""" +NOTE: This test has been disabled due to the removal of the /editor/journal route (see portality.view.editor#51) +""" + import pytest from doajtest import helpers diff --git a/portality/events/consumers/journal_editor_group_assigned_notify.py b/portality/events/consumers/journal_editor_group_assigned_notify.py index 6f1637d019..0119bcaeb3 100644 --- a/portality/events/consumers/journal_editor_group_assigned_notify.py +++ b/portality/events/consumers/journal_editor_group_assigned_notify.py @@ -48,6 +48,8 @@ def consume(cls, event): notification.short = svc.short_notification(cls.ID).format( issns=journal.bibjson().issns_as_text() ) - notification.action = url_for("editor.journal_page", journal_id=journal.id) + + # No action possible due to page removed (see portality.view.editor.journal_page(journal_id)) + # notification.action = url_for("editor.journal_page", journal_id=journal.id) svc.notify(notification) From eee82a2b47df590f08a71349ac3ccb3fdcf45987 Mon Sep 17 00:00:00 2001 From: Steven Eardley Date: Thu, 3 Oct 2024 15:19:02 +0100 Subject: [PATCH 2/3] Remove more references to disabled route --- .../test_application_processor_emails.py | 20 +++++++++---------- .../test_journal_assed_assigned_notify.py | 2 +- ...st_journal_editor_group_assigned_notify.py | 4 ++-- .../journal_assed_assigned_notify.py | 4 +++- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/doajtest/unit/application_processors/test_application_processor_emails.py b/doajtest/unit/application_processors/test_application_processor_emails.py index 894b4f30cc..cc106942e6 100644 --- a/doajtest/unit/application_processors/test_application_processor_emails.py +++ b/doajtest/unit/application_processors/test_application_processor_emails.py @@ -352,7 +352,7 @@ def test_01_maned_review_emails(self): assEd_email_matched = re.search(email_log_regex % (assEd_template, assEd_to, assEd_subject), info_stream_contents, re.DOTALL) - assert bool(assEd_email_matched) + assert bool(assEd_email_matched), info_stream_contents.strip('\x00') publisher_template = re.escape(templates.EMAIL_NOTIFICATION) publisher_to = re.escape(owner.email) @@ -401,7 +401,7 @@ def test_01_maned_review_emails(self): assEd_email_matched = re.search(email_log_regex % (assEd_template, assEd_to, assEd_subject), info_stream_contents, re.DOTALL) - assert bool(assEd_email_matched) + assert bool(assEd_email_matched), info_stream_contents.strip('\x00') assert len(re.findall(email_count_string, info_stream_contents)) == 2 # Clear the stream for the next part @@ -552,7 +552,7 @@ def test_02_ed_review_emails(self): assEd_email_matched = re.search(email_log_regex % (assEd_template, assEd_to, assEd_subject), info_stream_contents, re.DOTALL) - assert bool(assEd_email_matched) + assert bool(assEd_email_matched), info_stream_contents.strip('\x00') publisher_template = templates.EMAIL_NOTIFICATION publisher_to = re.escape(owner.email) @@ -590,7 +590,7 @@ def test_02_ed_review_emails(self): assEd_email_matched = re.search(email_log_regex % (assEd_template, assEd_to, assEd_subject), info_stream_contents, re.DOTALL) - assert bool(assEd_email_matched) + assert bool(assEd_email_matched), info_stream_contents.strip('\x00') assert len(re.findall(email_count_string, info_stream_contents)) == 1 # Clear the stream for the next part @@ -935,7 +935,7 @@ def test_01_maned_review_emails(self): assEd_email_matched = re.search(email_log_regex % (assEd_template, assEd_to, assEd_subject), info_stream_contents, re.DOTALL) - assert bool(assEd_email_matched) + assert bool(assEd_email_matched), info_stream_contents.strip('\x00') publisher_template = templates.EMAIL_NOTIFICATION publisher_to = re.escape(owner.email) @@ -985,7 +985,7 @@ def test_01_maned_review_emails(self): assEd_email_matched = re.search(email_log_regex % (assEd_template, assEd_to, assEd_subject), info_stream_contents, re.DOTALL) - assert bool(assEd_email_matched) + assert bool(assEd_email_matched), info_stream_contents.strip('\x00') assert len(re.findall(email_count_string, info_stream_contents)) == 2 # Clear the stream for the next part @@ -1124,7 +1124,7 @@ def test_02_ed_review_emails(self): assEd_email_matched = re.search(email_log_regex % (assEd_template, assEd_to, assEd_subject), info_stream_contents, re.DOTALL) - assert bool(assEd_email_matched) + assert bool(assEd_email_matched), info_stream_contents.strip('\x00') publisher_template = templates.EMAIL_NOTIFICATION publisher_to = re.escape(owner.email) @@ -1162,7 +1162,7 @@ def test_02_ed_review_emails(self): assEd_email_matched = re.search(email_log_regex % (assEd_template, assEd_to, assEd_subject), info_stream_contents, re.DOTALL) - assert bool(assEd_email_matched) + assert bool(assEd_email_matched), info_stream_contents.strip('\x00') assert len(re.findall(email_count_string, info_stream_contents)) == 1 # Clear the stream for the next part @@ -1347,7 +1347,7 @@ def test_01_maned_review_emails(self): assEd_email_matched = re.search(email_log_regex % (assEd_template, assEd_to, assEd_subject), info_stream_contents, re.DOTALL) - assert bool(assEd_email_matched) + assert bool(assEd_email_matched), info_stream_contents.strip('\x00') assert len(re.findall(email_count_string, info_stream_contents)) == 2 ctx.pop() @@ -1378,7 +1378,7 @@ def test_02_ed_review_emails(self): assEd_email_matched = re.search(email_log_regex % (assEd_template, assEd_to, assEd_subject), info_stream_contents, re.DOTALL) - assert bool(assEd_email_matched) + assert bool(assEd_email_matched), info_stream_contents.strip('\x00') assert len(re.findall(email_count_string, info_stream_contents)) == 1 ctx.pop() diff --git a/doajtest/unit/event_consumers/test_journal_assed_assigned_notify.py b/doajtest/unit/event_consumers/test_journal_assed_assigned_notify.py index 2882e51ea7..f24a9c894a 100644 --- a/doajtest/unit/event_consumers/test_journal_assed_assigned_notify.py +++ b/doajtest/unit/event_consumers/test_journal_assed_assigned_notify.py @@ -49,7 +49,7 @@ def test_consume_success(self): assert n.classification == constants.NOTIFICATION_CLASSIFICATION_ASSIGN assert n.long is not None assert n.short is not None - assert n.action is not None + assert n.action is None # view.editor.journal_page has been removed assert not n.is_seen() def test_consume_fail(self): diff --git a/doajtest/unit/event_consumers/test_journal_editor_group_assigned_notify.py b/doajtest/unit/event_consumers/test_journal_editor_group_assigned_notify.py index ed0b46d775..e55d0ea082 100644 --- a/doajtest/unit/event_consumers/test_journal_editor_group_assigned_notify.py +++ b/doajtest/unit/event_consumers/test_journal_editor_group_assigned_notify.py @@ -41,7 +41,7 @@ def test_consume_success(self): eg.set_editor("editor") eg.save(blocking=True) - event = models.Event(constants.EVENT_JOURNAL_EDITOR_GROUP_ASSIGNED, context={"journal" : app.data}) + event = models.Event(constants.EVENT_JOURNAL_EDITOR_GROUP_ASSIGNED, context={"journal": app.data}) JournalEditorGroupAssignedNotify.consume(event) time.sleep(1) @@ -54,7 +54,7 @@ def test_consume_success(self): assert n.classification == constants.NOTIFICATION_CLASSIFICATION_ASSIGN assert n.long is not None assert n.short is not None - assert n.action is not None + assert n.action is None # view.editor.journal_page has been removed assert not n.is_seen() def test_consume_fail(self): diff --git a/portality/events/consumers/journal_assed_assigned_notify.py b/portality/events/consumers/journal_assed_assigned_notify.py index 5788334e44..3fa2030d8a 100644 --- a/portality/events/consumers/journal_assed_assigned_notify.py +++ b/portality/events/consumers/journal_assed_assigned_notify.py @@ -43,6 +43,8 @@ def consume(cls, event): notification.short = svc.short_notification(cls.ID).format( issns=journal.bibjson().issns_as_text() ) - notification.action = url_for("editor.journal_page", journal_id=journal.id) + + # No action possible due to page removed (see portality.view.editor.journal_page(journal_id)) + # notification.action = url_for("editor.journal_page", journal_id=journal.id) svc.notify(notification) From 02f50eeccdc0d4af4fba342761e6ebf61b612eae Mon Sep 17 00:00:00 2001 From: Steven Eardley Date: Thu, 3 Oct 2024 16:10:54 +0100 Subject: [PATCH 3/3] Correct the method for skipping tests --- .../test_readonly_journal.py | 61 +++++++++++++++++-- ...ion_forms.py => test_application_forms.py} | 1 + 2 files changed, 56 insertions(+), 6 deletions(-) rename doajtest/unit/{_disabled_test_application_forms.py => test_application_forms.py} (95%) diff --git a/doajtest/unit/application_processors/test_readonly_journal.py b/doajtest/unit/application_processors/test_readonly_journal.py index 9e4bc4c42a..3a98d69805 100644 --- a/doajtest/unit/application_processors/test_readonly_journal.py +++ b/doajtest/unit/application_processors/test_readonly_journal.py @@ -39,14 +39,63 @@ def tearDown(self): super(TestReadOnlyJournal, self).tearDown() lcc.lookup_code = self.old_lookup_code - ########################################################### - # Tests on the publisher's re-journal form - ########################################################### - def test_01_readonly_journal_success(self): - """Give the read-only journal form a full workout""" + def test_01_unknown_context(self): + """ Pulling the wrong context gives an exception """ + + with self.assertRaises(AttributeError): + formulaic_context = JournalFormFactory.context("readonly") + fc = formulaic_context.processor(source=models.Journal(**JOURNAL_SOURCE)) + + def test_02_editor_readonly_journal(self): + """ Tests on the editor's read-only journal form """ + + # we start by constructing it from source + formulaic_context = JournalFormFactory.context("editor_readonly") + fc = formulaic_context.processor(source=models.Journal(**JOURNAL_SOURCE)) + assert isinstance(fc, ReadOnlyJournal) + assert fc.form is not None + assert fc.source is not None + assert fc.form_data is None + + # now construct it from form data (with a known source) + journal_obj = models.Journal(**JOURNAL_SOURCE) + journal_bibjson_obj = journal_obj.bibjson() + fc = formulaic_context.processor( + formdata=JOURNAL_FORM, + source=journal_obj + ) + + assert isinstance(fc, ReadOnlyJournal) + assert fc.form is not None + assert fc.source is not None + assert fc.form_data is not None + + # see that form has the correct info from an object (after all, that's the only point of having the form) + assert fc.form.title.data == journal_bibjson_obj.title + assert fc.form.pissn.data == journal_bibjson_obj.pissn + assert fc.form.eissn.data == journal_bibjson_obj.eissn + + # test each of the workflow components individually ... + + # run the validation + assert fc.validate(), fc.form.errors + + # run the crosswalk (no need to look in detail, xwalks are tested elsewhere) + fc.form2target() + assert fc.target is None # can't edit data using this form + + # patch the target with data from the source + fc.patch_target() + assert fc.target is None # can't edit data using this form + + # shouldn't be able to finalise, can't edit data using this form + self.assertRaises(Exception, fc.finalise) + + def test_03_maned_readonly_journal(self): + """ Tests on the managing editor's read-only journal form """ # we start by constructing it from source - formulaic_context = JournalFormFactory.context("readonly") + formulaic_context = JournalFormFactory.context("admin_readonly") fc = formulaic_context.processor(source=models.Journal(**JOURNAL_SOURCE)) assert isinstance(fc, ReadOnlyJournal) assert fc.form is not None diff --git a/doajtest/unit/_disabled_test_application_forms.py b/doajtest/unit/test_application_forms.py similarity index 95% rename from doajtest/unit/_disabled_test_application_forms.py rename to doajtest/unit/test_application_forms.py index 41a46cfb5b..7e0fd9e47e 100644 --- a/doajtest/unit/_disabled_test_application_forms.py +++ b/doajtest/unit/test_application_forms.py @@ -42,6 +42,7 @@ def test_disable_edit_note_except_editing_user(user_id, expected_result): class TestEditableNote(DoajTestCase): + @pytest.mark.skip(reason="Untestable: we don't have the editor.journal_page route enabled") # FIXME: permanently? def test_note_textarea_disabled_correctly(self): pwd = 'password123' acc = models.Account(**AccountFixtureFactory.make_editor_source())