Authors and editors can edit funder metadata #4061

ajrbyers · 2024-03-28T10:19:05Z

Closes #3164 (sort of)

Adds a new funding statement field
Updates the submission page for funding to have intro text, make use of a django form and validate it
Updates the editor edit_metadata interface to do the same
Adds a new edit view that is accessible to editors/staff at all times and to authors during submission
Adds a spinner to inform the user when a search is underway (previously there was no way to know if a seach was underway).
Additionally, after discussion, it was decided to rework the article/funder relationship to make it 1 Article to Many ArticleFunding entries. Two migrations take care of this process by looping over existing entries and adding the Article FK then renaming the model and removing the m2m. A method for Article called funders will return a queryset for backwards compatibility.

Screen recording: https://github.com/BirkbeckCTP/janeway/assets/8321378/bdf5bc3c-d78a-439e-81a2-e98a3ad679c8

mauromsl

Nice changes overall but I have a fundamental comment on the data model, that could imply further changes on the interface.

src/submission/models.py

mauromsl

Nice, thanks for the refactor, less tech debt moving forward

I added a minor suggestion on the print statement you have on the migration

src/journal/views.py

src/submission/migrations/0076_auto_20240402_1301.py

joemull

Overall this is really good, and I think people will really appreciate the flexibility. I especially love the status indicator while the user is waiting for search results to return from the API.

As for changes that need to be made, I found one server error and a handful of accessibility and consistency fixes we can make.

src/journal/views.py

src/templates/admin/submission/edit/funder.html

src/templates/admin/submission/submit_funding.html

src/templates/admin/elements/fundref/fundref.html

src/submission/views.py

src/submission/forms.py

…with article from 1-2-m

…nd change the modal and form element IDs

ajrbyers · 2024-04-03T21:31:18Z

Overall this is really good, and I think people will really appreciate the flexibility. I especially love the status indicator while the user is waiting for search results to return from the API.

As for changes that need to be made, I found one server error and a handful of accessibility and consistency fixes we can make.

Good catch on the error, I totally missed that (as a result of making changes to the model after adding the view). I've made some updates and cut down on the JS used by cloning the modal then updating the ID and FOR attributes. I was going to suggest opening a new issue to refactor this as its I think its beyond the scope of this issue but I gave it a go and got sucked down the rabbit hole.

ajrbyers · 2024-04-03T21:41:11Z

FYI haven't re-requested review as I'm going to take a look at refactoring the first block of JS also. Might as well keep it consistent.

ajrbyers · 2024-04-15T16:09:34Z

In the end I was able to cut the JS in half.

joemull

In the end I was able to cut the JS in half.

Nice. I tried to test it out but I got a migration conflict:

CommandError: Conflicting migrations detected; multiple leaf nodes in the migration graph: (0075_auto_20240312_0922, 0077_auto_20240402_1326 in submission).
To fix them run 'python manage.py makemigrations --merge'

ajrbyers · 2024-04-16T12:53:52Z

In the end I was able to cut the JS in half.

Nice. I tried to test it out but I got a migration conflict:
CommandError: Conflicting migrations detected; multiple leaf nodes in the migration graph: (0075_auto_20240312_0922, 0077_auto_20240402_1326 in submission).
To fix them run 'python manage.py makemigrations --merge'

I've added a merge for this.

joemull

Nice to have all the forms loading the same way.

The buttons weren't working in one of the places I tested, unfortunately. More inline about that.

Also, can the following line be removed for clarity or is something depending on it, like the custom submission fields? I am asking mainly because I want to make sure I know where all the "Add funder" buttons are so I can test them with the new JS.

https://github.com/BirkbeckCTP/janeway/blob/d5b09cfb9d4a2d0542c4b197483e964715002a60/src/templates/admin/submission/submit_info.html#L130

src/templates/admin/elements/fundref/fundref.html

joemull

It is now fully working for me! And the refactor is also much cleaner.

joemull · 2024-04-18T12:07:22Z

src/templates/admin/elements/fundref/fundref.html

-    </script>
+            });
+
+            // Clear the HTML, display the table and reinitialize foundation


We can get rid of this comment as it's outdated, right?

joemull · 2024-04-18T12:09:09Z

Also, can the following line be removed for clarity or is something depending on it, like the custom submission fields? I am asking mainly because I want to make sure I know where all the "Add funder" buttons are so I can test them with the new JS.

https://github.com/BirkbeckCTP/janeway/blob/d5b09cfb9d4a2d0542c4b197483e964715002a60/src/templates/admin/submission/submit_info.html#L130

Oh one last thing @ajrbyers . Did you see my question above?

ajrbyers · 2024-04-18T12:20:21Z

Also, can the following line be removed for clarity or is something depending on it, like the custom submission fields? I am asking mainly because I want to make sure I know where all the "Add funder" buttons are so I can test them with the new JS.
https://github.com/BirkbeckCTP/janeway/blob/d5b09cfb9d4a2d0542c4b197483e964715002a60/src/templates/admin/submission/submit_info.html#L130

Oh one last thing @ajrbyers . Did you see my question above?

Thanks @joemull agreed, much cleaner. The include is perhaps a bit confusing given its extension is .html, its actually including this file: https://github.com/BirkbeckCTP/janeway/blob/deadd395261775624680530816ce05938d7d8988/src/templates/admin/elements/fundref/fundref.html

I think because he wanted to use Django template tags Martin gave it the .html extension. For clarity maybe we should change it to .js?

joemull · 2024-04-18T12:41:18Z

@ajrbyers I'm not talking about the file extension but that I don't think it should be here at all. This is the submit_info page, which doesn't offer any funder interface, so far as I know:

https://github.com/BirkbeckCTP/janeway/blob/deadd395261775624680530816ce05938d7d8988/src/templates/admin/submission/submit_info.html#L130

Seems to me that the only places we need it are these two:

https://github.com/BirkbeckCTP/janeway/blob/deadd395261775624680530816ce05938d7d8988/src/templates/admin/submission/edit/metadata.html#L322

https://github.com/BirkbeckCTP/janeway/blob/deadd395261775624680530816ce05938d7d8988/src/templates/admin/submission/submit_funding.html#L150

Am I missing something?

ajrbyers · 2024-04-18T12:42:45Z

@ajrbyers I'm not talking about the file extension but that I don't think it should be here at all. This is the submit_info page, which doesn't offer any funder interface, so far as I know:

https://github.com/BirkbeckCTP/janeway/blob/deadd395261775624680530816ce05938d7d8988/src/templates/admin/submission/submit_info.html#L130

Seems to me that the only places we need it are these two:

https://github.com/BirkbeckCTP/janeway/blob/deadd395261775624680530816ce05938d7d8988/src/templates/admin/submission/edit/metadata.html#L322

https://github.com/BirkbeckCTP/janeway/blob/deadd395261775624680530816ce05938d7d8988/src/templates/admin/submission/submit_funding.html#L150

Am I missing something?

Nah you're not missing anything, I am though. Its not required on submit_info at all.

…be include no longer needed.

ajrbyers requested a review from mauromsl March 28, 2024 10:23

mauromsl requested changes Apr 2, 2024

View reviewed changes

src/submission/models.py Show resolved Hide resolved

ajrbyers requested a review from mauromsl April 2, 2024 12:59

mauromsl approved these changes Apr 2, 2024

View reviewed changes

src/journal/views.py Outdated Show resolved Hide resolved

src/submission/migrations/0076_auto_20240402_1301.py Outdated Show resolved Hide resolved

mauromsl requested a review from joemull April 2, 2024 14:12

joemull changed the title ~~Updated the Funder model to include an optional funding statement and added a new view to allow authors/editors to alter existing funding entries.~~ Authors and editors can edit funder metadata Apr 3, 2024

joemull requested changes Apr 3, 2024

View reviewed changes

ajrbyers added 6 commits April 3, 2024 17:39

#3164 various updates to the funder metadata system

a319aea

Renames the Funder models to ArticleFunding and changes relationship …

0ccf79d

…with article from 1-2-m

Added suggested changes.

6716de0

Adds fixes per code review.

c668cdd

Removed a lot of verbose JS in favour of cloning the existing modal a…

49ad576

…nd change the modal and form element IDs

Adds section to update the cloned form's label's for attr.

1d3d71d

ajrbyers force-pushed the 3164-funding_update branch from 1cb54e0 to 1d3d71d Compare April 3, 2024 21:19

Swapped var for let

de4b982

StephDriver assigned ajrbyers Apr 5, 2024

Completes a rewrite of the fundref API JS.

ae0945f

ajrbyers requested a review from joemull April 15, 2024 16:09

ajrbyers assigned joemull and unassigned ajrbyers Apr 15, 2024

joemull requested changes Apr 16, 2024

View reviewed changes

#3164 added migration merge.

d5b09cf

ajrbyers requested a review from joemull April 16, 2024 12:53

joemull requested changes Apr 17, 2024

View reviewed changes

src/templates/admin/elements/fundref/fundref.html Outdated Show resolved Hide resolved

Reworks the search_fundref function.

deadd39

ajrbyers requested a review from joemull April 17, 2024 15:05

joemull approved these changes Apr 18, 2024

View reviewed changes

joemull assigned ajrbyers and unassigned joemull Apr 18, 2024

Removed include from submit_info, removed old comment, removed varial…

dd10b59

…be include no longer needed.

ajrbyers assigned joemull and unassigned joemull Apr 18, 2024

ajrbyers merged commit 5d5a115 into master Apr 18, 2024
1 check failed

ajrbyers deleted the 3164-funding_update branch April 18, 2024 13:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Authors and editors can edit funder metadata #4061

Authors and editors can edit funder metadata #4061

ajrbyers commented Mar 28, 2024 •

edited

Loading

mauromsl left a comment

mauromsl left a comment

joemull left a comment

ajrbyers commented Apr 3, 2024 •

edited

Loading

ajrbyers commented Apr 3, 2024

ajrbyers commented Apr 15, 2024

joemull left a comment

ajrbyers commented Apr 16, 2024

joemull left a comment

joemull left a comment

joemull Apr 18, 2024

joemull commented Apr 18, 2024

ajrbyers commented Apr 18, 2024

joemull commented Apr 18, 2024

ajrbyers commented Apr 18, 2024

Authors and editors can edit funder metadata #4061

Authors and editors can edit funder metadata #4061

Conversation

ajrbyers commented Mar 28, 2024 • edited Loading

mauromsl left a comment

Choose a reason for hiding this comment

mauromsl left a comment

Choose a reason for hiding this comment

joemull left a comment

Choose a reason for hiding this comment

ajrbyers commented Apr 3, 2024 • edited Loading

ajrbyers commented Apr 3, 2024

ajrbyers commented Apr 15, 2024

joemull left a comment

Choose a reason for hiding this comment

ajrbyers commented Apr 16, 2024

joemull left a comment

Choose a reason for hiding this comment

joemull left a comment

Choose a reason for hiding this comment

joemull Apr 18, 2024

Choose a reason for hiding this comment

joemull commented Apr 18, 2024

ajrbyers commented Apr 18, 2024

joemull commented Apr 18, 2024

ajrbyers commented Apr 18, 2024

ajrbyers commented Mar 28, 2024 •

edited

Loading

ajrbyers commented Apr 3, 2024 •

edited

Loading