Feature/3573 autocheck article uploaded #2377

philipkcl · 2024-04-15T15:35:07Z

[3573] A check in the journal record to say whether or not articles have been uploaded

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 #

https://github.com/DOAJ/doajPM/issues/3573

Describe the scope/purpose of the PR here in as much detail as you like

Categorisation

This PR...

Basic PR Checklist

Instructions for developers:

For each checklist item, if it is N/A to your PR check the N/A box
For each item that you have done and confirmed for yourself, check Developer box (including if you have checked the N/A box)

Instructions for reviewers:

For each checklist item that has been confirmed by the Developer, check the Reviewer box if you agree
For multiple reviewers, feel free to add your own checkbox with your github username next to it if that helps with review tracking

Code Style

No deprecated methods are used
- N/A
- Developer
- Reviewer
No magic strings/numbers - all strings are in constants or messages files
- N/A
- Developer
- Reviewer
ES queries are wrapped in a Query object rather than inlined in the code
- N/A
- Developer
- Reviewer
Where possible our common library functions have been used (e.g. dates manipulated via dates)
- N/A
- Developer
- Reviewer
Cleaned up commented out code, etc
- N/A
- Developer
- Reviewer
Urls are constructed with url_for not hard-coded
- N/A
- Developer
- Reviewer

Testing

Unit tests have been added/modified
- N/A
- Developer
- Reviewer
Functional tests have been added/modified
- N/A
- Developer
- Reviewer
Code has been run manually in development, and functional tests followed locally
- N/A
- Developer
- Reviewer
Have CSS/style changes been implemented? If they are of a global scope (e.g. on base HTML elements) have the downstream impacts of the change in other areas of the system been considered?
- N/A
- Developer
- Reviewer

Documentation

FeatureMap annotations have been added
- N/A
- Developer
- Reviewer
Documentation updates - if needed - have been identified and prepared for inclusion into main documentation (e.g. added and highlighted/commented as appropriate to this PR)
- N/A
- Developer
- Reviewer
Core model documentation has been added to if needed: https://docs.google.com/spreadsheets/d/1lun2S9vwGbyfy3WjIjgXBm05D-3wWDZ4bp8xiIYfImM/edit
- N/A
- Developer
- Reviewer
Events and consumers documentation has been added if needed: https://docs.google.com/spreadsheets/d/1oIeG5vg-blm2MZCE-7YhwulUlSz6TOUeY8jAftdP9JE/edit
- N/A
- Developer
- Reviewer
The docs for this branch have been generated and pushed to the doc site (see docs/README.md for details)
- N/A
- Developer
- Reviewer

Release Readiness

If needed, migration has been created and tested locally
- N/A
- Developer
- Reviewer
Release sheet has been created, and completed as far as is possible https://docs.google.com/spreadsheets/d/1Bqx23J1MwXzjrmAygbqlU3YHxN1Wf7zkkRv14eTVLZQ/edit
- N/A
- Developer
- Reviewer
There has been a recent merge up from develop (or other base branch). List the dates of the merges up from develop below
- 2024-04-15

Testing

List the Functional Tests that must be run to confirm this feature

...
...

Deployment

What deployment considerations are there? (delete any sections you don't need)

Configuration changes

What configuration changes are included in this PR, and do we need to set specific values for production

Scripts

What scripts need to be run from the PR (e.g. if this is a report generating feature), and when (once, regularly, etc).

Migrations

What migrations need to be run to deploy this

Monitoring

What additional monitoring is required of the application as a result of this feature

New Infrastructure

What new infrastructure does this PR require (e.g. new services that need to run on the back-end).

Continuous Integration

What CI changes are required for this

amdomanska · 2024-04-19T10:12:49Z

The test should say "The journal may have fulfilled all the criteria for the Seal" (rather than has). We also need that text, together with the article info inside the ".doaj_seal__container form__question" div for stylistic reasons. You may need to change this in "editorial_form_fields.html"

Actually - wouldn't that be easier to just add this information to the existing paragraph and pass the required information to the template rather than creating the whole new widget and calling an ajax request?

doaj/portality/templates/application_form/editorial_form_fields.html

Line 59 in 6dd2ed5

<p>The journal may have fulfilled all the criteria for the Seal. </p>

amdomanska · 2024-04-19T10:03:30Z

portality/static/js/formulaic.js

+                fetch(`/admin/journal/${journalId}/article-info`)
+                    .then(response => response.json())
+                    .then(data => {
+                        const $p = $(sealSelector).prev('p');


This paragraph should be added INSIDE the sealSelector div, not before it.

philipkcl · 2024-04-19T11:57:21Z

The test should say "The journal may have fulfilled all the criteria for the Seal" (rather than has). We also need that text, together with the article info inside the ".doaj_seal__container form__question" div for stylistic reasons. You may need to change this in "editorial_form_fields.html"

Actually - wouldn't that be easier to just add this information to the existing paragraph and pass the required information to the template rather than creating the whole new widget and calling an ajax request?

doaj/portality/templates/application_form/editorial_form_fields.html

Line 59 in 6dd2ed5

<p>The journal may have fulfilled all the criteria for the Seal. </p>

@amdomanska,
In original and Dom's new requirement, the message are outside of .doaj_seal__container form__question.

https://github.com/DOAJ/doajPM/issues/3573#issuecomment-2049708516

If we still want to make that title inside .doaj_seal__container form__question, could you advise how to update the programme to achieve that by just update editorial_form_fields.html ?

you might need to discuss with @richard-jones about ajax request.

amdomanska · 2024-04-22T14:26:03Z

In original and Dom's new requirement, the message are outside of .doaj_seal__container form__question.

Absolutely, I'm aware of that. It appears to be an oversight that needs correction as we proceed with this task. It seems that the best approach, upon further consideration, would be to integrate it as a for the respective question. This way, it would naturally reside within the div, aligning seamlessly with the existing structure.

Feel free to proceed with updating the implementation in the "application_forms.py" file accordingly.

doaj/portality/forms/application_forms.py

Line 1604 in 6dd2ed5

DOAJ_SEAL = {

Regarding the approach to handle Ajax calls, I think there might be an alternative method worth considering. Instead of making additional calls, we could potentially streamline the process by utilizing the data already available from the backend through wtforms. For instance, in the template, you have access to "obj.data["admin"]" which could be passed directly to the form. Moreover, you might explore leveraging Python functions in "application_forms.py" to facilitate this integration.

Unfortunately, I'm currently focused on other tasks and can't provide a detailed walkthrough at the moment. However, if you encounter difficulties, feel free to assign the task to me, and I'll be more than happy to assist when I have the bandwidth.

@richard-jones thoughts?

richard-jones

A couple of minor, non blocking, style comments. Just requires addressing @amdomanska 's comments.

richard-jones · 2024-04-22T14:19:26Z

portality/forms/application_forms.py

@@ -1881,7 +1884,7 @@ class FieldDefinitions:
        "entry_template": "application_form/_entry_group.html",
        "widgets": [
            {"infinite_repeat" : {"enable_on_repeat" : ["textarea"]}},
-            "note_modal"
+            "note_modal",


extraneous comma added

richard-jones · 2024-04-22T14:19:45Z

doajtest/fixtures/accounts.py

@@ -83,7 +83,11 @@ def create_publisher_a():
    return publisher


-def create_maned_a():
+def create_maned_a(is_save=False):


probably just call this arg save

richard-jones · 2024-04-22T14:26:13Z

portality/forms/application_forms.py

+        ],
+        "widgets": [
+            "article_info",
+        ],


Probably worth adding a comment here that the article_info back-end is only accessible to admins. The DOAJ_SEAL also only appears in the admin form context, so that's all good, but I suppose we could change that at some point and not notice.

…heck_article_uploaded

amdomanska

You committed some changes to edges submodule, is that intentional? If not, that should be remove from the PR. Other than that looks good.

philipkcl · 2024-07-30T14:16:51Z

@amdomanska , there have no change in edges submodule, the change showed in PR seem like it comparing to old commit 990f4220163a3e18880f0bdc3ad5c80d234d22dd, I don't why it comparing that commit, may be just develop branch is pointing on it

amdomanska · 2024-07-31T08:53:50Z

@philipkcl To fix it you would have to merge develop up and update the submodule to make sure that no unintentional edges changes are committed (there should be no difference in edges between develop and your branch if you don't change it).

…heck_article_uploaded

…234d22dd

philipkcl added 7 commits April 15, 2024 11:46

draft articleinfo

b0dfc6b

implement ArticleInfo

edcd98a

update dictionary

c90cda0

setup article_info widgets

dcd962a

implement query of journal_article_info

969c687

add checking before init

3dcc38d

add test cases

96ac2eb

philipkcl requested review from richard-jones and Steven-Eardley April 15, 2024 15:35

amdomanska requested changes Apr 19, 2024

View reviewed changes

wording

d2f234d

merge develop 240419

f5c4c8e

philipkcl requested a review from amdomanska April 19, 2024 12:04

richard-jones approved these changes Apr 22, 2024

View reviewed changes

philipkcl added 4 commits May 1, 2024 13:15

naming

9532667

use multiple checkbox for styles only

e52ca45

Merge remote-tracking branch 'origin/develop' into feature/3573_autoc…

628ecf4

…heck_article_uploaded

update checkbox title

f415303

amdomanska approved these changes May 20, 2024

View reviewed changes

Steven-Eardley and others added 7 commits June 18, 2024 14:01

Merge branch 'develop' into feature/3573_autocheck_article_uploaded

b1daabf

fix for seal form field

ee1b642

avoid send request by invalid id

56a56ea

support in_doaj for count_by_issns query

f768d1b

change ArticleInfo result layout

5e9f946

support admin_site_search redirection

c8f3e73

merge develop 240708

a05f6f6

philipkcl requested a review from richard-jones July 8, 2024 13:12

philipkcl requested a review from amdomanska July 8, 2024 13:12

amdomanska approved these changes Jul 30, 2024

View reviewed changes

Reverted submodule edges to develop_edges1

ad0c19c

philipkcl requested a review from amdomanska July 30, 2024 14:16

philipkcl added 2 commits July 31, 2024 10:03

Merge remote-tracking branch 'origin/develop' into feature/3573_autoc…

161a013

…heck_article_uploaded

change edges HEAD to same as develop 990f4220163a3e18880f0bdc3ad5c80d…

c12c2ab

…234d22dd

amdomanska approved these changes Jul 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/3573 autocheck article uploaded #2377

Feature/3573 autocheck article uploaded #2377

philipkcl commented Apr 15, 2024 •

edited

Loading

amdomanska commented Apr 19, 2024

amdomanska Apr 19, 2024

philipkcl commented Apr 19, 2024 •

edited

Loading

amdomanska commented Apr 22, 2024

richard-jones left a comment

richard-jones Apr 22, 2024

richard-jones Apr 22, 2024

richard-jones Apr 22, 2024

amdomanska left a comment

philipkcl commented Jul 30, 2024

amdomanska commented Jul 31, 2024

Feature/3573 autocheck article uploaded #2377

Are you sure you want to change the base?

Feature/3573 autocheck article uploaded #2377

Conversation

philipkcl commented Apr 15, 2024 • edited Loading

[3573] A check in the journal record to say whether or not articles have been uploaded

Categorisation

Basic PR Checklist

Code Style

Testing

Documentation

Release Readiness

Testing

Deployment

Configuration changes

Scripts

Migrations

Monitoring

New Infrastructure

Continuous Integration

amdomanska commented Apr 19, 2024

amdomanska Apr 19, 2024

Choose a reason for hiding this comment

philipkcl commented Apr 19, 2024 • edited Loading

amdomanska commented Apr 22, 2024

richard-jones left a comment

Choose a reason for hiding this comment

richard-jones Apr 22, 2024

Choose a reason for hiding this comment

richard-jones Apr 22, 2024

Choose a reason for hiding this comment

richard-jones Apr 22, 2024

Choose a reason for hiding this comment

amdomanska left a comment

Choose a reason for hiding this comment

philipkcl commented Jul 30, 2024

amdomanska commented Jul 31, 2024

philipkcl commented Apr 15, 2024 •

edited

Loading

philipkcl commented Apr 19, 2024 •

edited

Loading