-
Notifications
You must be signed in to change notification settings - Fork 99
add coldfront core setting for PROJECT_UPDATE_FIELDS #737
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
base: main
Are you sure you want to change the base?
add coldfront core setting for PROJECT_UPDATE_FIELDS #737
Conversation
This allows for center directors to configure which Project fields project managers are able to edit. Signed-off-by: Cecilia Lau <[email protected]>
Tested and this works fine. @aebruno please look carefully at this one. I was able to create a weird situation where I was able as a PI to directly change the status of the project to archived without getting a warning first. There might be other odd situations that arise if certain fields are editable like this. |
Admittedly I only wanted/expected for this to take away editable fields (for us, title and field of science), and didn't consider the possibility of issues arising from adding other fields. |
Yeah it is up to the center director to mess this one up so I don't believe it is too big a deal. Maybe it would be better overall if ColdFront did more validation when directors configure custom settings. This would be a really big a totally separate issue for another date. This problem could be alleviated to by just tweaking this slightly to just having certain fields being removed by the setting. |
I agree that ColdFront doing more validation for directors configuring settings is a much bigger issue. I'm afraid I don't quite understand what you mean by having certain fields being removed by the setting - to disallow removing certain fields from being edited, or to disallow certain fields from being editable/added to the list? |
Sorry I could have phrased that better. Basically I'm just saying that instead of having There is probably a better way to express the logic but something like: normally_editable_fields = [
"title",
"description",
"field_of_science",
]
fields = [f for f in normally_editable_fields if f not in PROJECT_UPDATE_FIELDS_PI_CANNOT_UPDATE] would make it so that the setting only removes fields. But there might still be valid use cases where an admin might want to allow more fields to be editable. Unfortunately it is very busy right now at CCR so Andrew probably won't be able to look at this for a couple weeks anyways. |
Ah, I understand. Personally, I think I'd rather have the flexibility as an admin to add fields as well as remove, and I think that the responsibility to configure the app correctly should lie with the directors. |
This allows for center directors to configure which Project fields project managers are able to edit.
Closes #736.