diff --git a/CHANGELOG.md b/CHANGELOG.md index d937c10..ee1341b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ There is a generated client in `client`. It can be built with `clitools.py`. I'm starting to wire up the UI to use that. I took the NG submission-core and submission-ui and put them in the same project -and then refactored the pacakges. +and then refactored the packages. # 2024-09-24 Start of UI Brian Caruso @@ -73,8 +73,8 @@ without flask. This will need to be merged to arxiv-base master soon. ### Current state *WORKING* - creates a submission -- uplaod and unpacks tar.gz -- license, policy and author attestation +- upload and unpacks tar.gz +- submission agreement, and license acceptance - metadata: title abstract etc - docker file - tests diff --git a/submit_ce/implementations/compile/compile_at_gcp.py b/submit_ce/implementations/compile/compile_at_gcp.py index df5182f..d2c8bea 100644 --- a/submit_ce/implementations/compile/compile_at_gcp.py +++ b/submit_ce/implementations/compile/compile_at_gcp.py @@ -22,7 +22,7 @@ from typing import List, Optional from enum import Enum -from common import (GCP_LOG_NAME, GCP_RESULTS_NAME, GCP_PREFLIGHT_NAME, +from .common import (GCP_LOG_NAME, GCP_RESULTS_NAME, GCP_PREFLIGHT_NAME, DEFAULT_SUBMISSION_LOG_NAME, DEFAULT_SYSTEM_LOG_NAME, DEFAULT_COMPILATION_TIMEOUT, DEFAULT_MAX_APPEND_FILES, DEFAULT_MAX_TEX_FILES, MAX_RETRIES, RETRY_DELAY) @@ -478,7 +478,7 @@ def compile_submission( query_params['watermark_text'] = watermark_text if preflight: - query_params['preflight'] = args.preflight + query_params['preflight'] = preflight if not tex2pdf_url: raise FileNotFoundError("The tex2pdf_url is required. ") diff --git a/submit_ce/implementations/compile/compile_at_gcp_service.py b/submit_ce/implementations/compile/compile_at_gcp_service.py index 866a657..8fcd380 100644 --- a/submit_ce/implementations/compile/compile_at_gcp_service.py +++ b/submit_ce/implementations/compile/compile_at_gcp_service.py @@ -8,8 +8,7 @@ from submit_ce.api.CompileService import CompileService from submit_ce.api.domain.event.process import Result from submit_ce.api.domain.process import ProcessStatus -import sys -sys.path.append('submit_ce/implementations/compile') + from submit_ce.implementations.compile.compile_at_gcp import PreflightOption, DEFAULT_MAX_APPEND_FILES, DEFAULT_MAX_TEX_FILES, \ DEFAULT_COMPILATION_TIMEOUT, compile_submission @@ -81,4 +80,5 @@ def start_compile(self, submission: Submission, def check(self, process_id: str, user: User, client: Client) -> ProcessStatus: - pass \ No newline at end of file + pass + diff --git a/submit_ce/ui/controllers/__init__.py b/submit_ce/ui/controllers/__init__.py index df99673..1907139 100644 --- a/submit_ce/ui/controllers/__init__.py +++ b/submit_ce/ui/controllers/__init__.py @@ -10,7 +10,6 @@ from . import cross, delete, jref, util, withdraw from .manage_submissions import manage_submissions from .new import process, upload, review -from .new.authorship import authorship from .new.classification import classification from .new.create import create from .new.final import finalize diff --git a/submit_ce/ui/controllers/new/authorship.py b/submit_ce/ui/controllers/new/authorship.py deleted file mode 100644 index 8528365..0000000 --- a/submit_ce/ui/controllers/new/authorship.py +++ /dev/null @@ -1,122 +0,0 @@ -""" -Controller for authorship action. - -Creates an event of type `core.events.event.ConfirmAuthorship` -""" - -from http import HTTPStatus as status -from typing import Tuple, Dict, Any - -from arxiv.auth.auth import scopes -from arxiv.auth.domain import Session - -from arxiv.forms import csrf - -from flask import current_app -from werkzeug.datastructures import MultiDict -from wtforms import BooleanField, RadioField -from wtforms.validators import InputRequired, ValidationError, optional - -from submit_ce.api.domain.event import ConfirmAuthorship - -from submit_ce.ui.auth import user_and_client_from_session -from submit_ce.ui.controllers.util import validate_command -from submit_ce.ui.routes.flow_control import ready_for_next -from submit_ce.ui.backend import get_submission - -Response = Tuple[Dict[str, Any], int, Dict[str, Any]] # pylint: disable=C0103 - - -def authorship(method: str, params: MultiDict, session: Session, - submission_id: int, **kwargs) -> Response: - """Handle the authorship assertion view.""" - submitter, client = user_and_client_from_session(session) - submission, _ = get_submission(submission_id) - may_proxy = scopes.PROXY_SUBMISSION in submitter.scopes - - # The form should be prepopulated based on the current state of the submission. - if method == 'GET': - match submission.submitter_is_author, may_proxy: - case True, False: # already did form, is author - params['authorship'] = AuthorshipForm.YES - case True, True: # already did form - params['authorship'] = AuthorshipForm.YES - params['proxy'] = False - case False, True: # already did form and set False while being a proxy - params['authorship'] = AuthorshipForm.NO - params['proxy'] = True - case False, False: # bad state: not proxy and marked not author - params['authorship'] = False - case _, _: - pass # default values from form - - form = AuthorshipProxyForm(params) if may_proxy else AuthorshipForm(params) - - response_data = { - 'submission_id': submission_id, - 'form': form, - 'submission': submission, - 'submitter': submitter, - 'client': client, - 'may_proxy': may_proxy, - } - - if method == "GET": - return response_data, status.OK, {} - if method != "POST": - return response_data, status.METHOD_NOT_ALLOWED, {} - if not form.validate(): - return response_data, status.BAD_REQUEST, {} - - value = form.authorship.data == form.YES - not_yet_done = submission.submitter_is_author != value - command = ConfirmAuthorship(creator=submitter, client=client, - submitter_is_author=value) - if not_yet_done and validate_command(form, command, submission, 'authorship'): - submission, _ = current_app.api.save(command, submission_id=submission_id) - response_data['submission'] = submission - - return ready_for_next((response_data, status.OK, {})) - - - -class AuthorshipForm(csrf.CSRFForm): - """Generate form with radio button to confirm authorship information.""" - - YES = 'y' - NO = 'n' - - authorship = RadioField(choices=[(YES, 'I am submitting as an author of this article'), - (NO, 'I am not an author but have obtained pre-authorization' - 'from arXiv to submit as a third-party submitter')], - validators=[InputRequired('Please choose one')], - default=None) - - def validate_authorship(self, field: RadioField) -> None: - """Require proxy field if submitter is not author.""" - if field.data == self.NO: - # TODO could use better not author message - raise ValidationError('You must be the author of the paper you want to submit.') - - -class AuthorshipProxyForm(csrf.CSRFForm): - """Generate form with radio button to confirm authorship information.""" - - YES = 'y' - NO = 'n' - - authorship = RadioField(choices=[(YES, 'I am submitting as an author of this article'), - (NO, 'I am not an author but have obtained pre-authorization' - 'from arXiv to submit as a third-party submitter')], - validators=[InputRequired('Please choose one')], - default=YES) - proxy = BooleanField('By checking this box, I certify that I have ' - 'received authorization from arXiv to submit papers ' - 'on behalf of the author(s).', - validators=[optional()]) - - def validate_authorship(self, field: RadioField) -> None: - """Require proxy field if submitter is not author.""" - if field.data == self.NO and not self.data.get('proxy'): - raise ValidationError('You must get prior approval to submit ' - 'on behalf of authors') diff --git a/submit_ce/ui/controllers/new/tests/test_authorship.py b/submit_ce/ui/controllers/new/tests/test_authorship.py deleted file mode 100644 index 4f4094a..0000000 --- a/submit_ce/ui/controllers/new/tests/test_authorship.py +++ /dev/null @@ -1,45 +0,0 @@ -"""Tests for :mod:`submit_ce.controllers.authorship`.""" - -from submit_ce.ui.tests.csrf_util import parse_csrf_token - -def get(appx, subx): - with appx.app_context(): - return appx.api.get(subx.submission_id) - -def test_authorship_form(app, authorized_client, sub_verified_user): - sub = sub_verified_user - assert sub and not sub.submitter_is_author - - url = f"/{sub.submission_id}/authorship" - resp = authorized_client.get(url) - assert resp.status_code == 200 and b"Confirm Authorship" in resp.data and b"
Confirm Authorship" in resp.data and b"is-danger" in resp.data - assert not get(app, sub).submitter_is_author - resp = authorized_client.post(url, data={'action':'next','csrf_token':parse_csrf_token(resp)}) - assert resp.status_code == 400 and b"Confirm Authorship" in resp.data and b"is-danger" in resp.data - assert not get(app, sub).submitter_is_author - resp = authorized_client.post(url, data={'action':'next','csrf_token':parse_csrf_token(resp), 'authorship': 'n'}) - assert resp.status_code == 400 and b"<title>Confirm Authorship" in resp.data and b"is-danger" in resp.data - assert not get(app, sub).submitter_is_author - resp = authorized_client.post(url, data={'action':'next','csrf_token':parse_csrf_token(resp), 'authorship': 'bad_value'}) - assert resp.status_code == 400 and b"<title>Confirm Authorship" in resp.data and b"is-danger" in resp.data - assert not get(app, sub).submitter_is_author - resp = authorized_client.post(url, data={'action':'next','csrf_token':parse_csrf_token(resp), 'authorship': ''}) - assert resp.status_code == 400 and b"<title>Confirm Authorship" in resp.data and b"is-danger" in resp.data - assert not get(app, sub).submitter_is_author - - resp = authorized_client.post(url, data={'action':'next','csrf_token':parse_csrf_token(resp), 'authorship': 'y'}) - assert resp.status_code == 303 and resp.headers["Location"] == f"/{sub.submission_id}/policy" - assert get(app, sub).submitter_is_author - - -def test_no_submission(authorized_client): - url = "/2343/authorship" - resp = authorized_client.get(url) - assert resp.status_code == 404 - - url = "/totaljunkid/authorship" - resp = authorized_client.get(url) - assert resp.status_code == 404 diff --git a/submit_ce/ui/controllers/new/tests/test_license_noop_post.py b/submit_ce/ui/controllers/new/tests/test_license_noop_post.py new file mode 100644 index 0000000..188e2f5 --- /dev/null +++ b/submit_ce/ui/controllers/new/tests/test_license_noop_post.py @@ -0,0 +1,43 @@ +"""Tests for :mod:`submit_ce.controllers.license`.""" +import re +import pytest +from arxiv.license import LICENSES + + +def _extract_csrf(html_bytes: bytes) -> str: + # Adjust to your CSRF field name; typical hidden input name is "csrf_token" + m = re.search(rb'name="csrf_token" value="([^"]+)"', html_bytes) + return m.group(1).decode() if m else "" + +@pytest.mark.usefixtures("app") +def test_license_post_no_change_advances(authorized_client, sub_policy): + """ + When the user submits the same license choice that the submission already has, + the controller should treat it as a no-op and move to the next stage + (ready_for_next), returning 200 without error. + """ + sub = sub_policy + + # Add a license + first_uri = next(iter(LICENSES.keys())) + sub.license = type("L", (), {"uri": first_uri}) + + assert sub.license and sub.license.uri, "Fixture must provide an existing license URI" + + # 1) GET the license form (captures CSRF if enabled) + get_url = f"/{sub.submission_id}/license" + get_resp = authorized_client.get(get_url) + assert get_resp.status_code == 200 + csrf_token = _extract_csrf(get_resp.data) + + # 2) POST the *same* license value (no-op) + post_data = { + "license": sub.license.uri, # same as current license + } + if csrf_token: + post_data["csrf_token"] = csrf_token + + post_resp = authorized_client.post(get_url, data=post_data, follow_redirects=False) + + # Expected: controller returns a 200 OK and advances via ready_for_next + assert post_resp.status_code == 200 diff --git a/submit_ce/ui/controllers/new/tests/test_verify_user.py b/submit_ce/ui/controllers/new/tests/test_verify_user.py index f63be4c..6dbeb21 100644 --- a/submit_ce/ui/controllers/new/tests/test_verify_user.py +++ b/submit_ce/ui/controllers/new/tests/test_verify_user.py @@ -30,4 +30,4 @@ def sub_from_db(): "verify_user": "true", "action":"next"}) assert resp.status_code == 303 and \ - resp.headers["Location"] == f"/{sub.submission_id}/authorship" + resp.headers["Location"] == f"/{sub.submission_id}/policy" diff --git a/submit_ce/ui/controllers/new/tests/test_verify_user_normal.py b/submit_ce/ui/controllers/new/tests/test_verify_user_normal.py new file mode 100644 index 0000000..d75fff8 --- /dev/null +++ b/submit_ce/ui/controllers/new/tests/test_verify_user_normal.py @@ -0,0 +1,33 @@ +"""Tests for :mod:`submit_ce.controllers.verify_user`.""" +import pytest +from http import HTTPStatus as status +from submit_ce.ui.tests.csrf_util import parse_csrf_token +from submit_ce.api.domain.submission import Submission + +@pytest.mark.usefixtures("app") +def test_verify_user_normal(authorized_client, sub_created): + """ + Non-proxy submitter: GET -> POST verify_user=true should advance (ready_for_next). + Covers validate(), save(), and ready_for_next path. + """ + sub: Submission = sub_created + assert not sub.submitter_contact_verified + + url = f"/{sub.submission_id}/verify_user" + + # GET form + resp_get = authorized_client.get(url) + assert resp_get.status_code == status.OK + + # Real CSRF from page + token = parse_csrf_token(resp_get) + + # POST verify_user=true + resp_post = authorized_client.post( + url, + data={"verify_user": "y", "csrf_token": token}, + follow_redirects=False + ) + + # Depending on your flow_control wrapper, this might be 200 or a redirect + assert resp_post.status_code in (status.OK, status.FOUND, status.SEE_OTHER) diff --git a/submit_ce/ui/controllers/new/tests/test_verify_user_proxy.py b/submit_ce/ui/controllers/new/tests/test_verify_user_proxy.py new file mode 100644 index 0000000..7a3a249 --- /dev/null +++ b/submit_ce/ui/controllers/new/tests/test_verify_user_proxy.py @@ -0,0 +1,58 @@ +"""Tests for :mod:`submit_ce.controllers.verify_user`.""" +import pytest +from http import HTTPStatus as status +from submit_ce.ui.tests.csrf_util import parse_csrf_token +from submit_ce.api.domain.submission import Submission + +@pytest.mark.usefixtures("app") +def test_verify_user_proxy_requires_fields(authorized_client, sub_created, monkeypatch): + """ + Proxy submitter: first POST missing fields -> 400; second POST with fields -> advance. + Covers may_proxy validation branch and both error/success paths. + """ + sub: Submission = sub_created + assert not sub.submitter_contact_verified + url = f"/{sub.submission_id}/verify_user" + + # give the current authorized user the PROXY_SUBMISSION scope + from arxiv.auth.auth import scopes + from submit_ce.ui import auth as auth_mod + # Patch only for the duration of this test: always return a user with PROXY_SUBMISSION + user_client = auth_mod.user_and_client_from_session + def fake_user_and_client(session): + submitter, client = user_client(session) + # ensure PROXY_SUBMISSION is present + submitter.scopes = set(getattr(submitter, "scopes", [])) | {scopes.PROXY_SUBMISSION} + return submitter, client + + monkeypatch.setattr( + "submit_ce.ui.controllers.new.verify_user.user_and_client_from_session", + fake_user_and_client + ) + + # GET form + resp_get = authorized_client.get(url) + assert resp_get.status_code == status.OK + token = parse_csrf_token(resp_get) + + # 1) Test bad request: POST with verify_user=true but missing proxy fields -> BAD_REQUEST + resp_bad = authorized_client.post( + url, + data={"verify_user": "y", "csrf_token": token}, + follow_redirects=False + ) + assert resp_bad.status_code == status.BAD_REQUEST + + # 2) Test good request: POST with verify_user=true and valid proxy fields -> should advance + resp_ok = authorized_client.post( + url, + data={ + "verify_user": "y", + "proxy_name": "Jane Proxy", + "proxy_email": "jane.proxy@example.org", + "csrf_token": token, + }, + follow_redirects=False + ) + assert resp_ok.status_code in (status.OK, status.FOUND, status.SEE_OTHER) + diff --git a/submit_ce/ui/controllers/new/verify_user.py b/submit_ce/ui/controllers/new/verify_user.py index 2aab91c..593841e 100644 --- a/submit_ce/ui/controllers/new/verify_user.py +++ b/submit_ce/ui/controllers/new/verify_user.py @@ -8,10 +8,11 @@ from flask import current_app from werkzeug.datastructures import MultiDict -from wtforms import BooleanField -from wtforms.validators import InputRequired +from wtforms import BooleanField, StringField +from wtforms.validators import InputRequired, Optional, Email, Length import logging +from arxiv.auth.auth import scopes from arxiv.forms import csrf from arxiv.auth.domain import Session @@ -39,6 +40,7 @@ def verify(method: str, params: MultiDict, session: Session, # Will raise NotFound if there is no such submission. submission, _ = get_submission(submission_id) + may_proxy = scopes.PROXY_SUBMISSION in submitter.scopes if method == 'GET' and submission.submitter_contact_verified: params['verify_user'] = 'true' @@ -50,6 +52,7 @@ def verify(method: str, params: MultiDict, session: Session, 'submission': submission, 'submitter': submitter, 'user': session.user, # We want the most up-to-date representation. + 'may_proxy': may_proxy, } if method == "GET": @@ -59,7 +62,18 @@ def verify(method: str, params: MultiDict, session: Session, if not form.validate() or not form.verify_user.data: return stay_on_this_stage((response_data, status.BAD_REQUEST, {})) - + + if may_proxy: + ok = True + if not (form.proxy_name.data or "").strip(): + form.proxy_name.errors.append("Proxy for name is required.") + ok = False + if not (form.proxy_email.data or "").strip(): + form.proxy_email.errors.append("Proxy for e‑mail is required.") + ok = False + if not ok: + return stay_on_this_stage((response_data, status.BAD_REQUEST, {})) + if submission.submitter_contact_verified: return ready_for_next((response_data, status.OK,{})) @@ -79,3 +93,8 @@ class VerifyUserForm(csrf.CSRFForm): 'I confirm that my contact information is correct', [InputRequired('Please confirm your user information')], ) + + proxy_name = StringField('Proxy for name', + validators=[Optional(), Length(max=200)]) + proxy_email = StringField('Proxy for e-mail', + validators=[Optional(), Email("Enter a valid e-mail address.")]) diff --git a/submit_ce/ui/controllers/tests/test_jref_get_announced.py b/submit_ce/ui/controllers/tests/test_jref_get_announced.py new file mode 100644 index 0000000..47ff64d --- /dev/null +++ b/submit_ce/ui/controllers/tests/test_jref_get_announced.py @@ -0,0 +1,42 @@ +"""Tests for :mod:`submit_ce.controllers.jref` - test for announced submission.""" +import pytest +from werkzeug.datastructures import MultiDict +from submit_ce.api.domain.agent import InternalClient +from submit_ce.ui.controllers.jref import jref # jref function + +@pytest.mark.usefixtures("app") +def test_jref_get_announced_returns_prepopulated_form(monkeypatch, authorized_user): + class Meta: + doi = "10.1/old" + journal_ref = "Old JR" + report_num = "RN-1" + + class Sub: + is_announced = True + metadata = Meta() + + # Backend + auth patches in the controller namespace + monkeypatch.setattr( + "submit_ce.ui.controllers.jref.get_submission", + lambda sid: (Sub(), []) + ) + monkeypatch.setattr( + "submit_ce.ui.controllers.jref.user_and_client_from_session", + lambda session: (authorized_user, InternalClient(name="test-client")) + ) + + # ❗Disable CSRF on the form class via module path (no need for request/session) + monkeypatch.setattr( + "submit_ce.ui.controllers.jref.JREFForm.Meta.csrf", + False, + raising=False, + ) + + data, code, _ = jref("GET", MultiDict(), object(), submission_id=42) + + assert code == 200 + assert "form" in data + form = data["form"] + assert form.doi.data == "10.1/old" + assert form.journal_ref.data == "Old JR" + assert form.report_num.data == "RN-1" diff --git a/submit_ce/ui/controllers/tests/test_jref_post_confirmed_save.py b/submit_ce/ui/controllers/tests/test_jref_post_confirmed_save.py new file mode 100644 index 0000000..729ebd3 --- /dev/null +++ b/submit_ce/ui/controllers/tests/test_jref_post_confirmed_save.py @@ -0,0 +1,71 @@ +"""Tests for :mod:`submit_ce.controllers.jref` - confirmed changes trigger save.""" +import pytest +from types import SimpleNamespace +from unittest import mock +from werkzeug.datastructures import MultiDict +from http import HTTPStatus as status + +from flask import current_app +from submit_ce.ui.controllers.jref import jref +from submit_ce.api.domain.agent import InternalClient + + +@pytest.mark.usefixtures("app") +def test_jref_post_confirmed_changed_saves_and_redirects(monkeypatch, authorized_user, app): + # Build a submission that is announced with old metadata + class Meta: + doi = "10.1/old"; journal_ref = "Old JR"; report_num = "RN-1" + class Sub: + is_announced = True + metadata = Meta() + + # Return real domain objects, not strings + monkeypatch.setattr( + "submit_ce.ui.controllers.jref.user_and_client_from_session", + lambda session: (authorized_user, InternalClient(name="test-client")) + ) + + # Controller-namespace patches + monkeypatch.setattr("submit_ce.ui.controllers.jref.get_submission", lambda sid: (Sub(), [])) + monkeypatch.setattr("submit_ce.ui.controllers.jref.url_for", lambda *a, **k: "/url/for/create-submission") + monkeypatch.setattr("submit_ce.ui.controllers.jref.alerts.flash_success", lambda *a, **k: None) + + # Disable CSRF / make form validate() + monkeypatch.setattr("arxiv.forms.csrf.get_application_config", + lambda: {"CSRF_SECRET": "test-secret"}, raising=False) + req_stub = SimpleNamespace( + auth=None, + session=SimpleNamespace(nonce="test-nonce", session_id="sid-1"), + remote_addr="127.0.0.1", + ) + monkeypatch.setattr("arxiv.forms.csrf.request", req_stub, raising=False) + monkeypatch.setattr("submit_ce.ui.controllers.jref.JREFForm.validate", lambda self: True) + + # validators return True to allow save to proceed + monkeypatch.setattr("submit_ce.ui.controllers.jref.validate_command", lambda *a, **k: True) + + # Fake save updates the submission metadata and returns (submission, events) + def _fake_save(*events, submission_id=None): + new = mock.MagicMock() + md = SimpleNamespace(doi="10.1/new", journal_ref="New JR", report_num="RN-2") + new.metadata = md + new.is_announced = True + return new, list(events) + + # Patch current_app.api inside an app context so the proxy resolves + with app.app_context(): + current_app.api = mock.MagicMock() + current_app.api.save.side_effect = _fake_save + + params = MultiDict({ + "doi": "10.1/new", + "journal_ref": "New JR", + "report_num": "RN-2", + "confirmed": "1", # True → proceed to save + }) + + data, code, headers = jref("POST", params, object(), submission_id=123) + + assert code == status.SEE_OTHER + assert headers.get("Location") == "/url/for/create-submission" + diff --git a/submit_ce/ui/controllers/tests/test_jref_post_noop.py b/submit_ce/ui/controllers/tests/test_jref_post_noop.py new file mode 100644 index 0000000..4b0feb5 --- /dev/null +++ b/submit_ce/ui/controllers/tests/test_jref_post_noop.py @@ -0,0 +1,59 @@ +"""Tests for :mod:`submit_ce.controllers.jref` - test that confirmed but unchanged is noop.""" +import pytest +from types import SimpleNamespace +from werkzeug.datastructures import MultiDict +from submit_ce.ui.controllers.jref import jref + +@pytest.mark.usefixtures("app") +def test_jref_post_confirmed_but_unchanged(monkeypatch): + class Meta: + doi = "10.9999/existing" + journal_ref = "Nucl.Phys.Proc.Suppl. 109 (2002) 3-9" + report_num = "SU-4240-720" + class Sub: is_announced=True; metadata=Meta() + + # Controller-namespace patches + monkeypatch.setattr("submit_ce.ui.controllers.jref.get_submission", + lambda sid: (Sub(), [])) + monkeypatch.setattr("submit_ce.ui.controllers.jref.user_and_client_from_session", + lambda session: ("creator", "client")) + + # CSRF pieces to allow form construction + monkeypatch.setattr( + "arxiv.forms.csrf.get_application_config", + lambda: {"CSRF_SECRET": "test-secret"}, + raising=False + ) + req_stub = SimpleNamespace( + auth=None, + session=SimpleNamespace(nonce="test-nonce", session_id="sid-1"), + remote_addr="127.0.0.1", + ) + monkeypatch.setattr("arxiv.forms.csrf.request", req_stub, raising=False) + + # Bypass CSRF/form validation for this unit test + monkeypatch.setattr( + "submit_ce.ui.controllers.jref.JREFForm.validate", + lambda self: True + ) + + # Guard: save must NOT be called on no-op post + class DummyAPI: + def save(self, *args, **kwargs): + raise AssertionError("save() must not be called for no-op POST") + class DummyApp: api = DummyAPI() + monkeypatch.setattr("submit_ce.ui.controllers.jref.current_app", DummyApp()) + + class Session: ... + params = MultiDict({ + "doi": Meta.doi, + "journal_ref": Meta.journal_ref, + "report_num": Meta.report_num, + "confirmed": "true", # True → skip confirmation branch + }) + + data, code, headers = jref("POST", params, Session(), submission_id=1) + + assert code == 200 + assert headers == {} + assert "require_confirmation" not in data diff --git a/submit_ce/ui/controllers/tests/test_jref_post_unconfirmed.py b/submit_ce/ui/controllers/tests/test_jref_post_unconfirmed.py new file mode 100644 index 0000000..cd04e12 --- /dev/null +++ b/submit_ce/ui/controllers/tests/test_jref_post_unconfirmed.py @@ -0,0 +1,54 @@ +"""Tests for :mod:`submit_ce.controllers.jref` - test for unconfirmed changed.""" +import pytest +from types import SimpleNamespace +from werkzeug.datastructures import MultiDict +from submit_ce.ui.controllers.jref import jref + +@pytest.mark.usefixtures("app") +def test_jref_post_valid_but_unconfirmed(monkeypatch): + class Meta: doi=None; journal_ref=None; report_num=None + class Sub: is_announced=True; metadata=Meta() + + # Patch where the controller looks these up + monkeypatch.setattr("submit_ce.ui.controllers.jref.get_submission", + lambda sid: (Sub(), [])) + monkeypatch.setattr("submit_ce.ui.controllers.jref.user_and_client_from_session", + lambda session: ("creator", "client")) + + # CSRF: provide config & request stub + monkeypatch.setattr( + "arxiv.forms.csrf.get_application_config", + lambda: {"CSRF_SECRET": "test-secret"}, + raising=False + ) + req_stub = SimpleNamespace( + auth=None, + session=SimpleNamespace(nonce="test-nonce", session_id="sid-1"), + remote_addr="127.0.0.1", + ) + monkeypatch.setattr("arxiv.forms.csrf.request", req_stub, raising=False) + + # Bypass CSRF/form validation for this unit test + monkeypatch.setattr( + "submit_ce.ui.controllers.jref.JREFForm.validate", + lambda self: True + ) + + # Guard: save() MUST NOT run when not confirmed + class DummyAPI: + def save(self, *args, **kwargs): + raise AssertionError("save() must not be called when not confirmed") + class DummyApp: api = DummyAPI() + monkeypatch.setattr("submit_ce.ui.controllers.jref.current_app", DummyApp()) + + class Session: ... + params = MultiDict({ + "doi": "10.1234/abcd", + "confirmed": "", # in BooleanField.false_values -> False + }) + + data, code, headers = jref("POST", params, Session(), submission_id=1) + + assert code == 200 + assert headers == {} + assert data.get("require_confirmation") is True diff --git a/submit_ce/ui/controllers/tests/test_jref_unannounced_get.py b/submit_ce/ui/controllers/tests/test_jref_unannounced_get.py new file mode 100644 index 0000000..c4dd990 --- /dev/null +++ b/submit_ce/ui/controllers/tests/test_jref_unannounced_get.py @@ -0,0 +1,45 @@ +"""Tests for :mod:`submit_ce.controllers.jref` - test unannounced redirect to create.""" +import pytest +from werkzeug.datastructures import MultiDict +from submit_ce.ui.controllers.jref import jref + +@pytest.mark.usefixtures("app") +def test_jref_get_unannounced_redirects(monkeypatch): + """ + If the submission is not announced, GET should flash a failure and + redirect (303) back to the create_submission page. + """ + + class Sub: + is_announced = False + metadata = None + + # Patch the controller's bindings to avoid backend and Flask context + monkeypatch.setattr( + "submit_ce.ui.controllers.jref.get_submission", + lambda sid: (Sub(), []) + ) + monkeypatch.setattr( + "submit_ce.ui.controllers.jref.user_and_client_from_session", + lambda session: ("creator", "client") + ) + # Avoid needing request/session context for flash + url_for + monkeypatch.setattr( + "submit_ce.ui.controllers.jref.alerts.flash_failure", + lambda *a, **k: None + ) + monkeypatch.setattr( + "submit_ce.ui.controllers.jref.url_for", + lambda endpoint, **kwargs: "/create" + ) + + class Session: ... + data, code, headers = jref( + method="GET", + params=MultiDict(), + session=Session(), + submission_id=1234 + ) + + assert code == 303 + assert headers.get("Location") == "/create" \ No newline at end of file diff --git a/submit_ce/ui/routes/ui.py b/submit_ce/ui/routes/ui.py index be250cc..acf3f71 100644 --- a/submit_ce/ui/routes/ui.py +++ b/submit_ce/ui/routes/ui.py @@ -295,14 +295,15 @@ def verify_user(submission_id: Optional[int] = None) -> Response: 'Verify User Information', submission_id, flow_controlled=True) -@UI.route('/<int:submission_id>/authorship', methods=['GET', 'POST']) +@UI.route('/<int:submission_id>/policy', methods=['GET', 'POST']) @scoped(scopes.EDIT_SUBMISSION, authorizer=is_owner, unauthorized=redirect_to_login) @flow_control() -def authorship(submission_id: int) -> Response: - """Render step 2, authorship.""" - return handle(cntrls.authorship, 'submit/authorship.html', - 'Confirm Authorship', submission_id, flow_controlled=True) +def policy(submission_id: int) -> Response: + """Render step 2, policy agreement.""" + return handle(cntrls.policy, 'submit/policy.html', + 'Acknowledge Policy Statement', submission_id, + flow_controlled=True) @UI.route('/<int:submission_id>/license', methods=['GET', 'POST']) @@ -315,23 +316,12 @@ def license(submission_id: int) -> Response: 'Select a License', submission_id, flow_controlled=True) -@UI.route('/<int:submission_id>/policy', methods=['GET', 'POST']) -@scoped(scopes.EDIT_SUBMISSION, authorizer=is_owner, - unauthorized=redirect_to_login) -@flow_control() -def policy(submission_id: int) -> Response: - """Render step 4, policy agreement.""" - return handle(cntrls.policy, 'submit/policy.html', - 'Acknowledge Policy Statement', submission_id, - flow_controlled=True) - - @UI.route('/<int:submission_id>/classification', methods=['GET', 'POST']) @scoped(scopes.EDIT_SUBMISSION, authorizer=is_owner, unauthorized=redirect_to_login) @flow_control() def classification(submission_id: int) -> Response: - """Render step 5, choose classification.""" + """Render step 4, choose classification.""" return handle(cntrls.classification, 'submit/classification.html', 'Choose a Primary Classification', submission_id, diff --git a/submit_ce/ui/templates/submit/policy.html b/submit_ce/ui/templates/submit/policy.html index f3ff208..b5ae479 100644 --- a/submit_ce/ui/templates/submit/policy.html +++ b/submit_ce/ui/templates/submit/policy.html @@ -1,58 +1,14 @@ {% extends "submit/base_edit.html" %} -{% block title -%}Acknowledge Policy Statement{%- endblock title %} +{% block title -%}Submission Agreement{%- endblock title %} +{% block page_class %}legal-page{% endblock page_class %} {% block within_content %} <form id="form" method="POST" action="{{ url_for('ui.policy', submission_id=submission_id) }}"> <div class="content-container"> <h2>Terms and Conditions - Scroll to read before proceeding</h2> <div class="policy-scroll"> - <p>Any person submitting a work for deposit in arXiv is required to assent to the following terms and conditions.</p> - - <h3>Representations and Warranties</h3> - <p>The Submitter makes the following representations and warranties:</p> - <ul> - <li>The Submitter is an original author of the Work, or a proxy pre-approved by arXiv administrators acting on behalf of an original author.</li> - <li>If the Work was created by multiple authors, that all of them have consented to the submission of the Work to arXiv.</li> - <li>The Submitter has the right to include any third-party materials used in the Work.</li> - <li>The use of any third-party materials is consistent with scholarly and academic standards of proper citation and acknowledgement of sources.</li> - <li>The Work is not subject to any agreement, license or other claim that could interfere with the rights granted herein.</li> - <li>The distribution of the Work by arXiv will not violate any rights of any person or entity, including without limitation any rights of copyright, patent, trade secret, privacy, or any other rights, and it contains no defamatory, obscene, or other unlawful matter.</li> - <li>The Submitter has authority to make the statements, grant the rights, and take the actions described above.</li> - </ul> - - <h3>Grant of the License to arXiv</h3> - <p>The Submitter grants to arXiv upon submission of the Work a non-exclusive, perpetual, and royalty-free license to include the Work in the arXiv repository and permit users to access, view, and make other uses of the work in a manner consistent with the services and objectives of arXiv. This License includes without limitation the right for arXiv to reproduce (e.g., upload, backup, archive, preserve, and allow online access), distribute (e.g., allow access), make available, and prepare versions of the Work (e.g., , abstracts, and metadata or text files, formats for persons with disabilities) in analog, digital, or other formats as arXiv may deem appropriate. - - <h3>Waiver of Rights and Indemnification</h3> - <p>The Submitter waives the following claims on behalf of him/herself and all other authors:</p> - <ul> - <li>Any claims against arXiv or Cornell University, or any officer, employee, or agent thereof, based upon the use of the Work by arXiv consistent with the License.</li> - <li>Any claims against arXiv or Cornell University, or any officer, employee, or agent thereof, based upon actions taken by any such party with respect to the Work, including without limitation decisions to include the Work in, or exclude the Work from, the repository; editorial decisions or changes affecting the Work, including the identification of Submitters and their affiliations or titles; the classification or characterization of the Work; the content of any metadata; the availability or lack of availability of the Work to researchers or other users of arXiv, including any decision to include the Work initially or remove or disable access.</li> - <li>Any rights related to the integrity of the Work under Moral Rights principles.</li> - <li>Any claims against arXiv or Cornell University, or any officer, employee, or agent thereof based upon any loss of content or disclosure of information provided in connection with a submission.</li> - <li>The Submitter will indemnify, defend, and hold harmless arXiv, Cornell University and its officers, employees, agents, and other affiliated parties from any loss, damage, or claim arising out of or related to: (a) any breach of any representations or warranties herein; (b) any failure by Submitter to perform any of Submitter’s obligations herein; and (c) any use of the Work as anticipated in the License and terms of submittal.</li> - </ul> - <h3>Management of Copyright</h3> - <p>This grant to arXiv is a non-exclusive license and is not a grant of exclusive rights or a transfer of the copyright. The Submitter (and any other authors) retains their copyright and may enter into publication agreements or other arrangements, so long as they do not conflict with the ability of arXiv to exercise its rights under the License. arXiv has no obligation to protect or enforce any copyright in the Work, and arXiv has no obligation to respond to any permission requests or other inquiries regarding the copyright in or other uses of the Work.</p> - - <p>The Submitter may elect to make the Work available under one of the following Creative Commons licenses that the Submitter shall select at the time of submission: - <ul> - <li>Creative Commons Attribution license (CC BY 4.0)</li> - <li>Creative Commons Attribution-ShareAlike license (CC BY-SA 4.0)</li> - <li>Creative Commons Attribution-Noncommercial-ShareAlike license (CC BY-NC-SA 4.0)</li> - <li>Creative Commons Public Domain Dedication (CC0 1.0)</li> - </ul> - - <p>If you wish to use a different CC license, then select arXiv's non-exclusive license to distribute in the arXiv submission process and indicate the desired Creative Commons license in the actual article. - The Creative Commons licenses are explained here:<br> - https://creativecommons.org/licenses/.</p> - - <h3>Metadata license</h3> - <p>To the extent that the Submitter or arXiv has a copyright interest in metadata accompanying the submission, a Creative Commons CC0 1.0 Universal Public Domain Dedication will apply. Metadata includes title, author, abstract, and other information describing the Work.</p> - - <h3>General Provisions</h3> - <p>This Agreement will be governed by and construed in accordance with the substantive and procedural laws of the State of New York and the applicable federal law of the United States without reference to any conflicts of laws principles. The Submitter agrees that any action, suit, arbitration, or other proceeding arising out of or related to this Agreement must be commenced in the state or federal courts serving Tompkins County, New York. The Submitter hereby consents on behalf of the Submitter and any other authors to the personal jurisdiction of such courts.</p> + <p>Updated Submission Agreement</p> </div> </div> diff --git a/submit_ce/ui/templates/submit/verify_user.html b/submit_ce/ui/templates/submit/verify_user.html index 29336ef..244cd4e 100644 --- a/submit_ce/ui/templates/submit/verify_user.html +++ b/submit_ce/ui/templates/submit/verify_user.html @@ -2,64 +2,143 @@ {% block title -%}Verify Your Contact Information{%- endblock title %} -{% block important_text_box -%} +{% block info_container_middle -%} + <div class="message-body"> - <p><span class="is-danger">*</span>A current email address is required to complete submission. Your email address is viewable to <a href="{{ url_for('help_email') }}">registered arXiv users</a>.</p> + <p><span class="is-danger">*</span>A current email address is required to complete submission. Your email address is + viewable to <a href="{{ url_for('help_email') }}">registered arXiv users</a>.</p> + <p>Complete and accurate authorship is required and will be + <a href="{{ url_for('help_author') }}">displayed in the public metadata</a>.</p> </div> -{%- endblock important_text_box %} +{%- endblock info_container_middle %} {% block within_content %} -<p>Review your contact information for accuracy or <a href="{{ url_for('account') }}">edit your contact information</a> before proceeding</p> -<form id="form" method="POST" action="{% if submission_id %}{{ url_for('ui.verify_user', submission_id=submission_id) }}{% else %}{{ url_for('ui.verify_user') }}{% endif %}"> - {{ form.csrf_token }} - <div class="field action-container verify-user-checkbox"> - {% if form.verify_user.errors %}<div class="notification is-danger">{% endif %} - <div class="control"> - <div class="checkbox"> - {{ form.verify_user }} - {{ form.verify_user.label }} - </div> - {% for error in form.verify_user.errors %} - <p class="help is-danger field-error">{{ error }}</p> - {% endfor %} +<p>Review your contact information for accuracy or <a href="{{ url_for('account') }}">edit your contact information</a> + before proceeding. + If you are under 18 years of age (minor) you must complete the + <a href="https://arxiv.org/help/arXivGuardianConsentSubmissionForm.pdf" target="_blank" rel="noopener"> + arXiv parent/guardian consent form + </a> + before proceeding to submit your Work. +</p> +<form id="form" method="POST" + action="{% if submission_id %}{{ url_for('ui.verify_user', submission_id=submission_id) }}{% else %}{{ url_for('ui.verify_user') }}{% endif %}"> + {{ form.csrf_token }} + <div class="field action-container verify-user-checkbox"> + {% if form.verify_user.errors %} + <div class="notification is-danger">{% endif %} + <div class="control"> + <div class="checkbox"> + {{ form.verify_user }} + {{ form.verify_user.label }} + </div> + {% for error in form.verify_user.errors %} + <p class="help is-danger field-error">{{ error }}</p> + {% endfor %} + </div> + {% if form.verify_user.errors %} + </div> + {% endif %} </div> {% if form.verify_user.errors %}</div>{% endif %} - </div> -</form> -<article class="user-info content-container verify-user-box"> - <div class="field is-horizontal"> - <div class="field-label"> - <span class="has-text-weight-semibold">First or given name(s):</span> - </div> - <div class="field-body"><span class="is-expanded">{{ user.name.forename }}</span></div> - </div> - <div class="field is-horizontal"> - <div class="field-label"> - <span class="has-text-weight-semibold">Last or family name(s):</span> - </div> - <div class="field-body">{{ user.name.surname }}</div> - </div> - <div class="field is-horizontal"> - <div class="field-label"> - <span class="has-text-weight-semibold">Suffix:</span> - </div> - <div class="field-body">{{ user.name.suffix }}</div> - </div> - <div class="field is-horizontal"> - <div class="field-label"> - <span class="has-text-weight-semibold">Affiliation:</span> - </div> - <div class="field-body">{{ user.profile.affiliation }}</div> - </div> - <div class="field is-horizontal"> - <div class="field-label"> - <span class="has-text-weight-semibold">E-mail:</span> + <article class="user-info content-container verify-user-box"> + <div class="field is-horizontal"> + <div class="field-label"> + <span class="has-text-weight-semibold">First or given name(s)</span> + </div> + <div class="field-body"><span class="is-expanded" id="submitter_firstname">{{ user.name.forename }}</span> + </div> + </div> + <div class="field is-horizontal"> + <div class="field-label"> + <span class="has-text-weight-semibold">Last or family name(s)</span> + </div> + <div class="field-body" id="submitter_lastname">{{ user.name.surname }}</div> + </div> + <div class="field is-horizontal"> + <div class="field-label"> + <span class="has-text-weight-semibold">Suffix</span> + </div> + <div class="field-body">{{ user.name.suffix }}</div> + </div> + <div class="field is-horizontal"> + <div class="field-label"> + <span class="has-text-weight-semibold">Affiliation</span> + </div> + <div class="field-body">{{ user.profile.affiliation }}</div> + </div> + <div class="field is-horizontal"> + <div class="field-label"> + <span class="has-text-weight-semibold">E-mail</span> + </div> + <div class="field-body" id="submitter_email" data-email="{{ user.email }}">{{ user.email }} + <span class="is-danger" aria-hidden="true">*</span></div> + </div> + </article> + + {% if may_proxy %} + <div class="content-container"> + + <h2>Proxy submitters only</h2> + + <div class="field"> + <div class="control"> + <label class="checkbox"> + <input type="checkbox" id="autofill_proxy"> + Check to fill in your own contact information (when submitting as yourself). + </label> + </div> + </div> + + <div class="field"> + <label class="label" for="proxy_name">Proxy for name</label> + {{ form.proxy_name(class_="input", id="proxy_name", autocomplete="name", + aria_describedby="proxy_name_help") }} + + {% for error in form.proxy_name.errors %} + <p class="help is-danger field-error">{{ error }}</p> + {% endfor %} + <p class="help" id="proxy_name_help">Enter the full name of the primary + author you are proxying for.</p> + </div> + + <div class="field"> + <label class="label" for="proxy_email">Proxy for e‑mail</label> + {{ form.proxy_email(class_="input", id="proxy_email", inputmode="email", autocomplete="email") }} + {% for error in form.proxy_email.errors %} + <p class="help is-danger field-error">{{ error }}</p> + {% endfor %} + </div> </div> - <div class="field-body">{{ user.email }} <span class="is-danger">*</span></div> - </div> -</article> + {% endif %} +</form> + +<script> + document.addEventListener("DOMContentLoaded", () => { + const box = document.getElementById("autofill_proxy"); + if (!box) return; + + const fnSpan = document.getElementById("submitter_firstname"); + const lnSpan = document.getElementById("submitter_lastname"); + const emailContainer = document.getElementById("submitter_email"); + + const proxyName = document.getElementById("proxy_name"); + const proxyEmail = document.getElementById("proxy_email"); + box.addEventListener("change", () => { + if (box.checked) { + const fullName = fnSpan.textContent.trim() + " " + + lnSpan.textContent.trim(); + proxyName.value = fullName; + proxyEmail.value = (emailContainer.dataset.email || "").trim(); + } else { + proxyName.value = ""; + proxyEmail.value = ""; + } + }); + }); +</script> {% endblock within_content %} diff --git a/submit_ce/ui/tests/test_submit_web.py b/submit_ce/ui/tests/test_submit_web.py index f3ee8ce..962b388 100644 --- a/submit_ce/ui/tests/test_submit_web.py +++ b/submit_ce/ui/tests/test_submit_web.py @@ -55,19 +55,6 @@ def _sub(): 'csrf_token': token}) assert response.status_code == status.SEE_OTHER - # Get the next page in the process. This is the authorship stage. - next_page = urlparse(response.headers['Location']) - assert 'authorship' in next_page.path - response = client.get(next_page.path) - assert response.status_code == status.OK - assert 'I am submitting as an author of this article' in response.text - - # Submit the authorship page. - response = client.post(next_page.path, data={'authorship': 'y', - 'action': 'next', - 'csrf_token': _parse_csrf_token(response)}) - assert response.status_code == status.SEE_OTHER - # Get the next page in the process. This is the policy stage. next_page = urlparse(response.headers['Location']) assert 'policy' in next_page.path diff --git a/submit_ce/ui/workflow/__init__.py b/submit_ce/ui/workflow/__init__.py index 72be6d6..ad22090 100644 --- a/submit_ce/ui/workflow/__init__.py +++ b/submit_ce/ui/workflow/__init__.py @@ -118,8 +118,7 @@ def get_stage(self, query: Union[type, Stage, str, int])\ 'NewSubmissionWorkflow', [ stages.VerifyUser(), - stages.Authorship(), - stages.Policy(), + stages.Agreement(), stages.License(), stages.Classification(), stages.FileUpload(), @@ -135,8 +134,7 @@ def get_stage(self, query: Union[type, Stage, str, int])\ ReplacementWorkflow = WorkflowDefinition( 'ReplacementWorkflow', [stages.VerifyUser(must_see=True), - stages.Authorship(must_see=True), - stages.Policy(must_see=True), + stages.Agreement(must_see=True), stages.License(must_see=True), stages.FileUpload(must_see=True), stages.ReviewFiles(must_see=True), diff --git a/submit_ce/ui/workflow/stages.py b/submit_ce/ui/workflow/stages.py index bafe121..02cfe9d 100644 --- a/submit_ce/ui/workflow/stages.py +++ b/submit_ce/ui/workflow/stages.py @@ -53,23 +53,13 @@ class VerifyUser(Stage): completed = [conditions.is_contact_verified] -class Authorship(Stage): - """The user is asked to verify their authorship status.""" - - endpoint = 'authorship' - label = 'confirm authorship' - title = "Confirm authorship" - display = "Authorship" - completed = [conditions.is_authorship_indicated] - - -class Policy(Stage): +class Agreement(Stage): """The user is required to agree to arXiv policies.""" endpoint = 'policy' label = 'accept arXiv submission policies' - title = "Acknowledge policy" - display = "Policy" + title = "Submission Agreement" + display = "Agreement" completed = [conditions.is_policy_accepted] @@ -78,7 +68,7 @@ class License(Stage): endpoint = 'license' label = 'choose a license' - title = "Choose license" + title = "Use of Author's Work" display = "License" completed = [conditions.has_license] diff --git a/submit_ce/ui/workflow/tests/test_new_submission.py b/submit_ce/ui/workflow/tests/test_new_submission.py index 4f66fac..77244f8 100644 --- a/submit_ce/ui/workflow/tests/test_new_submission.py +++ b/submit_ce/ui/workflow/tests/test_new_submission.py @@ -15,7 +15,7 @@ class TestNewSubmissionWorkflow(CtrlBase): def testWorkflowGetitem(self): wf = workflow.WorkflowDefinition( name='TestingWorkflow', - order=[VerifyUser(), Policy(), FinalPreview()]) + order=[VerifyUser(), Agreement(), FinalPreview()]) self.assertIsNotNone(wf[VerifyUser]) self.assertEqual(wf[VerifyUser].__class__, VerifyUser) @@ -26,7 +26,7 @@ def testWorkflowGetitem(self): self.assertEqual(wf[VerifyUser], wf['verify_user']) self.assertEqual(wf[VerifyUser], wf[wf.order[0]]) - self.assertEqual(next(wf.iter_prior(wf[Policy])), wf[VerifyUser]) + self.assertEqual(next(wf.iter_prior(wf[Agreement])), wf[VerifyUser]) def testVerifyUser(self): seen = {} @@ -43,7 +43,7 @@ def testVerifyUser(self): self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[Authorship])) self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[License])) - self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[Policy])) + self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[Agreement])) self.assertFalse(nswfps.can_proceed_to( nswfps.workflow[Classification])) self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[CrossList])) @@ -64,7 +64,7 @@ def testVerifyUser(self): self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Authorship])) self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[License])) - self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[Policy])) + self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[Agreement])) self.assertFalse(nswfps.can_proceed_to( nswfps.workflow[Classification])) self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[CrossList])) @@ -85,7 +85,7 @@ def testVerifyUser(self): self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Authorship])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[License])) - self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[Policy])) + self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[Agreement])) self.assertFalse(nswfps.can_proceed_to( nswfps.workflow[Classification])) self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[CrossList])) @@ -105,7 +105,7 @@ def testVerifyUser(self): self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[VerifyUser])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Authorship])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[License])) - self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Policy])) + self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Agreement])) self.assertFalse(nswfps.can_proceed_to( nswfps.workflow[Classification])) @@ -118,15 +118,15 @@ def testVerifyUser(self): self.assertFalse(nswfps.can_proceed_to(nswfps.workflow[FinalPreview])) self.assertFalse(nswfps.can_proceed_to(nswfps.workflow.confirmation)) - self.assertEqual(nswfps.current_stage(), nswfps.workflow[Policy]) + self.assertEqual(nswfps.current_stage(), nswfps.workflow[Agreement]) submission.submitter_accepts_policy = True - nswfps.mark_seen(nswfps.workflow[Policy]) + nswfps.mark_seen(nswfps.workflow[Agreement]) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[VerifyUser])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Authorship])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[License])) - self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Policy])) + self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Agreement])) self.assertTrue(nswfps.can_proceed_to( nswfps.workflow[Classification])) @@ -148,7 +148,7 @@ def testVerifyUser(self): self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[VerifyUser])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Authorship])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[License])) - self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Policy])) + self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Agreement])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Classification])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[CrossList])) # should be allowed to skip the "cross" but may be a problem with has_seen @@ -170,7 +170,7 @@ def testVerifyUser(self): self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[VerifyUser])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Authorship])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[License])) - self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Policy])) + self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Agreement])) self.assertTrue(nswfps.can_proceed_to( nswfps.workflow[Classification])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[CrossList])) @@ -192,7 +192,7 @@ def testVerifyUser(self): self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[VerifyUser])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Authorship])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[License])) - self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Policy])) + self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Agreement])) self.assertTrue(nswfps.can_proceed_to( nswfps.workflow[Classification])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[CrossList])) @@ -227,7 +227,7 @@ def testVerifyUser(self): self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[VerifyUser])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Authorship])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[License])) - self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Policy])) + self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Agreement])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Classification])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[CrossList])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[FileUpload])) @@ -253,7 +253,7 @@ def testVerifyUser(self): self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[VerifyUser])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Authorship])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[License])) - self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Policy])) + self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[Agreement])) self.assertTrue(nswfps.can_proceed_to( nswfps.workflow[Classification])) self.assertTrue(nswfps.can_proceed_to(nswfps.workflow[CrossList])) diff --git a/submit_ce/ui/workflow/tests/test_workflow_definition_api.py b/submit_ce/ui/workflow/tests/test_workflow_definition_api.py new file mode 100644 index 0000000..fb00bc8 --- /dev/null +++ b/submit_ce/ui/workflow/tests/test_workflow_definition_api.py @@ -0,0 +1,58 @@ +"""Test basic workflow definition and previous/current/next functions.""" +import pytest +from submit_ce.ui.workflow import WorkflowDefinition, NewSubmissionWorkflow +from submit_ce.ui.workflow.stages import Stage + +class A(Stage): + label = "A" + endpoint = "ep_a" # <-- add this + def is_complete(self, sub): return False + def incomplete(self, sub): return ["A incomplete"] + +class B(Stage): + label = "B" + endpoint = "ep_b" # <-- add this + def is_complete(self, sub): return True + def incomplete(self, sub): return [] + +class C(Stage): + label = "C" + endpoint = "ep_c" # <-- add this + def is_complete(self, sub): return True + def incomplete(self, sub): return [] + +@pytest.mark.usefixtures("app") +def test_workflow_definition_core_apis(): + wf = WorkflowDefinition( + name="WF", + order=[A(), B(), C()], + confirmation=C() + ) + + # __iter__ + assert [type(s).__name__ for s in wf] == ["A", "B", "C"] + + # iter_prior: yields up to but not including the target + assert [type(s).__name__ for s in wf.iter_prior(wf.order[2])] == ["A", "B"] + + # next_stage / previous_stage (including edges) + assert type(wf.next_stage(wf.order[0])).__name__ == "B" + assert wf.next_stage(wf.order[-1]) is None + assert wf.previous_stage(wf.order[0]) is None + assert type(wf.previous_stage(wf.order[2])).__name__ == "B" + + # __getitem__ / get_stage by int + assert type(wf[0]).__name__ == "A" + assert wf.get_stage(10) is None # out-of-range int + + # get_stage by class and by string (classname) + assert type(wf.get_stage(B)).__name__ == "B" + assert type(wf.get_stage("B")).__name__ == "B" + + # get_stage(): error when given non-Stage class + with pytest.raises(ValueError): + wf.get_stage(list) + + # slice returns list of Stage instances + sl = wf[0:2] + assert [type(s).__name__ for s in sl] == ["A", "B"] diff --git a/submit_ce/ui/workflow/tests/test_workflow_processor_core.py b/submit_ce/ui/workflow/tests/test_workflow_processor_core.py new file mode 100644 index 0000000..93e3dc7 --- /dev/null +++ b/submit_ce/ui/workflow/tests/test_workflow_processor_core.py @@ -0,0 +1,66 @@ +""" Additional workflow test, finalize and complete submission.""" +import pytest +from flask import current_app + +from submit_ce.ui.workflow import WorkflowDefinition +from submit_ce.ui.workflow.processor import WorkflowProcessor +from submit_ce.ui.workflow.stages import ( + VerifyUser, Agreement, License, Classification, + FileUpload, ReviewFiles, Process, Metadata, + OptionalMetadata, FinalPreview, Confirm +) + +from submit_ce.api.domain.agent import InternalClient +from submit_ce.api.domain.event import FinalizeSubmission + + +@pytest.mark.usefixtures("app") +def test_workflow_processor_paths(sub_metadata, authorized_user, app): + """ + Verify WorkflowProcessor: + - Picks the first incomplete real stage + - Reports complete only after FinalPreview (is_finalized) is satisfied + """ + + # Build a realistic workflow using existing stages (as in production) + wf = WorkflowDefinition( + "WF", + order=[ + VerifyUser(), + Agreement(), + License(), + Classification(), + FileUpload(), + ReviewFiles(), + Process(), + Metadata(), + OptionalMetadata(), + FinalPreview(), # <- this will be the first incomplete stage pre-finalization + Confirm(), # confirmation step + ], + confirmation=Confirm(), + ) + + submission = sub_metadata # Not finalized yet + + proc = WorkflowProcessor(workflow=wf, submission=submission) + + # Workflow should not be complete + assert proc.is_complete() is False + + # First incomplete stage should be FinalPreview, because all earlier + # stages' .completed predicates evaluate to True on sub_metadata. + assert type(proc.current_stage()).__name__ == "FinalPreview" + + # ---- Finalize Submission ---- + with app.app_context(): + creator = authorized_user + client = InternalClient(name="test-client") + submission, _ = current_app.api.save( + FinalizeSubmission(creator=creator, client=client), + submission_id=submission.submission_id, + ) + + # Now workflow should be complete + proc2 = WorkflowProcessor(workflow=wf, submission=submission) + assert proc2.is_complete() is True \ No newline at end of file