WC Admin application cycle edit/create table #590
Conversation
Implemented ModelForm for Wharton admin
backend/clubs/views.py
Outdated
| """ | ||
|
|
||
| permission_classes = [WhartonApplicationPermission | IsSuperuser] | ||
| http_method_names = ["get", "post", "put", "patch", "delete"] |
There was a problem hiding this comment.
Do we need put, patch, and delete?
Docstring only specifies update and list?
There was a problem hiding this comment.
Forgot to update the docstring, thanks for the catch! Default behavior for post and delete are used in ModelForm to let the user create delete cycles I believe, and patch is used for update.
There was a problem hiding this comment.
I meant more of:
- do we want to allow deleting?
- we should consider picking one of put/patch, they are generally considered to have similar semantics ("put" is idempotent, whereas "patch" is not always)
backend/clubs/views.py
Outdated
| app.application_end_time = self.get_object().end_date | ||
| if app.result_release_time < app.application_end_time: | ||
| app.result_release_time = app.application_end_time | ||
| +datetime.timedelta(days=10) |
There was a problem hiding this comment.
This seems to pass lint but looks a bit awkward? Definitely a nit though
backend/clubs/views.py
Outdated
| if app.result_release_time < app.application_end_time: | ||
| app.result_release_time = app.application_end_time | ||
| +datetime.timedelta(days=10) | ||
| app.save() |
There was a problem hiding this comment.
This won't affect prod performance but it would be good practice to use bulk_update!
|
Great work! Left a few comments but otherwise looks like this is a good start towards making administration of applications easier! Thanks Julian |
|
Refs issue #580 |
Implement bulk_update and clarify http_method_names Enforce ISO format on ModelForm date-time return values (@esinx)
| ` | ||
|
|
||
| const WhartonApplicationCycles = (): ReactElement => { | ||
| const [editMembership, setEditMembership] = React.useState(false) |
There was a problem hiding this comment.
Why not simply import useState from React?
| }) | ||
|
|
||
| const [editExtensions, setEditExtensions] = React.useState(false) | ||
| const [extensionsCycle, setExtensionsCycle] = React.useState({ |
There was a problem hiding this comment.
Can we type extensionCycle and membershipCycle?
| } | ||
| }, [extensionsCycle]) | ||
|
|
||
| if (clubOptionsMembership == null) { |
There was a problem hiding this comment.
Suggestion: We should try to use something like react-query or SWR to streamline loading process
|
Looks like we're pretty close to ready to merge this? @julianweng do you have any time to get this merged in before we start school? |
Implemented ModelForm on Wharton Council Admin page, connecting to WhartonCyclesView (enforces cycle start/end times on all applications with given cycle upon update).