Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow future discontinued date #2204

Closed
wants to merge 44 commits into from

Conversation

amdomanska
Copy link
Contributor

@amdomanska amdomanska commented Jan 30, 2023

Allow future discontinued date

Please don't delete any sections when completing this PR template; instead, enter N/A for checkboxes or sections which are not applicable unless otherwise stated below

See #1529

The current implementation allows future discontinued dates.

  • Added changes: hide discontinued date from the ToC if in the future
  • Send notifications if any journals with a discontinued date in the set future (parameterized in settings.py) are found

Currently added settings:
3. Checks for journals with a discontinued date today - settings.py DISCONTINUED_DATE_DELTA
4. background job run every day at 07:00 - settings.py HUEY_SCHEDULE["find_discontinued_soon"]

Categorisation

This PR...

  • has scripts to run
  • has migrations to run
  • adds new infrastructure
  • changes the CI pipeline
  • affects the public site
  • affects the editorial area
  • affects the publisher area
  • affects the monitoring

Basic PR Checklist

Testing

Public Site/ToC/Test Correctly Displayed Discontinued Date

Configuration changes (current settings to confirm)

Journals discontinuing in how many days (0) should be searched for?
DISCONTINUED_DATE_DELTA = 0
When and how often the background job should be run? (**everyday at 07:00 **)
HUEY_SCHEDULE["find_discontinued_soon"] = {"month": "*", "day": "*", "day_of_week": "*", "hour": "7", "minute": "0"}

@amdomanska amdomanska marked this pull request as draft January 30, 2023 11:50
@amdomanska amdomanska marked this pull request as ready for review February 1, 2023 14:20
@amdomanska
Copy link
Contributor Author

Conflict resolved.

Copy link
Contributor

@RK206 RK206 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small suggestion. May be it will be helpful to update Feature test to check on 'Ceased publication on ' text on TOC page

Copy link
Contributor

@richard-jones richard-jones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments from me, and there are also other requests for changes from other reviewers


NOTIFICATION_CLASSIFICATION_STATUS = "status"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we invent a new classification status, we need to think about how this is displayed in the notifications search UI. It will need an icon, or at least to check the edges code that shows the notifications search that it falls back to the text gracefully

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you suggest using one of the existing ones?

NOTIFICATION_CLASSIFICATION_STATUS_CHANGE = "status_change"
NOTIFICATION_CLASSIFICATION_ASSIGN = "assign"
NOTIFICATION_CLASSIFICATION_CREATE = "create"
NOTIFICATION_CLASSIFICATION_FINISHED = "finished"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, I think a new one makes sense, though "status" is a bit of an overloaded term. Perhaps "alert"? Then the icon can be a bell or something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To do this, set the status in the constants like you have done, for this new "alert", then in portality/static/js/edges/notifications.edge.js add a new SVG icon in the icons dictionary, and a new description in the classifications dictionary and this should then display as appropriate

portality/tasks/find_discontinued_soon.py Show resolved Hide resolved
Copy link
Contributor

@richard-jones richard-jones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one remaining comment from @amdomanska about cleaning something up at merge time, otherwise this is good to go to the test/release process

@amdomanska
Copy link
Contributor Author

There's one remaining comment from @amdomanska about cleaning something up at merge time, otherwise this is good to go to the test/release process

Done.

Copy link
Contributor

@Steven-Eardley Steven-Eardley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed a missing import which prevented the background job being run

Copy link
Contributor

@Steven-Eardley Steven-Eardley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amdomanska This branch has the following genuine test failures:

=========================== short test summary info ============================
FAILED doajtest/unit/test_task_discontinued_soon.py::TestDiscontinuedSoon::test_discontinued_soon_found - TypeError: unsupported type for timedelta days component: BackgroundJob
FAILED doajtest/unit/test_task_discontinued_soon.py::TestDiscontinuedSoon::test_discontinued_soon_not_found - TypeError: unsupported type for timedelta days component: BackgroundJob
FAILED doajtest/unit/api_tests/test_apiv3_discovery.py::TestAPIDiscovery::test_03_applications - AttributeError: module 'portality.lib.dates' has no attribute 'after'
========= 3 failed, 2128 passed, 82401 warnings in 3418.57s (0:56:58) ==========

Exited with code exit status 1

@amdomanska
Copy link
Contributor Author

Continued: #2239

@amdomanska amdomanska closed this May 4, 2023
@Steven-Eardley Steven-Eardley deleted the feature/1529_future_discontinued_date branch June 22, 2023 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants