HyphaApp / hypha

Submission management software for open calls
https://www.hypha.app
BSD 3-Clause "New" or "Revised" License
67 stars 38 forks source link

Convert `STAFF_UPLOAD_CONTRACT` to Wagtail setting #3953

Closed bickelj closed 1 week ago

bickelj commented 1 month ago

To make it easier for administrators to change the Project workflow, change the low-level setting STAFF_UPLOAD_CONTRACT into a Wagtail Project Setting staff_upload_contract.

Issue #3952

Test Steps

Before the change: only the Contracting role should be able to upload contracts to a project in Contracting status when system setting STAFF_UPLOAD_CONTRACT is False or not set. After the change: the same should be true but only when the Wagtail Project Setting staff_upload_contract is False or not set, system setting STAFF_UPLOAD_CONTRACT should have no effect.

frjo commented 1 month ago

This summer I plan to start an Epic to create a Hypa settings page in Wagtail admin and a custom Wagtail admin front page for Hypha.

There are a number of settings that should be moved from env vars to Wagtail settings. This makes them easier to change and make them more discoverable.

Therefore I will most likely not merge this is, instead waiting for the bigger solution.

bickelj commented 1 month ago

@frjo There are reasons to merge this one sooner.

  1. I am going to submit another PR that is related to and builds on this one. It adds a separate but related option that ends up grouped with this option in a MultiFieldPanel in Wagtail. It will be confusing to have one of these as a system setting and the other as a Wagtail option. It would also be moving backwards to have the new option be a system setting.
  2. You might reduce the burden of that epic by putting this one in now. It's ready to go.
wes-otf commented 1 month ago

@frjo I can take a look at this tomorrow - might take away some headaches of merge conflicts down the line to get this in sooner, especially if @bickelj is going to continue building on it. I do really like the idea of a wagtail epic this summer too! Let me know what you think.

frjo commented 1 month ago

The issue is that we do not plan to implement the settings as wagtail admin settings. They will show up there but be implemented more straight forward in Django.

There are some 20+ settings that should be moved. We find Wagtail admin settings a bit cumbersome to work with when we need a lot of them. See the amount of code in this PR for adding a single setting.

wes-otf commented 1 month ago

that makes sense, I'll hold off on testing then