Skip to content

Complete manage-admissions endpoint#877

Merged
norbye merged 1 commit intomasterfrom
manage-backend
Apr 17, 2023
Merged

Complete manage-admissions endpoint#877
norbye merged 1 commit intomasterfrom
manage-backend

Conversation

@norbye
Copy link
Copy Markdown
Member

@norbye norbye commented Mar 1, 2023

PR to enable the creation and editing of admissions from the website (should remove the need for shelling<3).

Tasks

  • Create django endpoints
  • Connect the frontend to the endpoints
  • Connect groups to the endpoint
  • Add visual feedback to the frontend

Noteworthy stuffs

  • Webkom + the leaders of HS, revue and backup have access to create admissions
  • Webkom can edit any admission, the others can edit admissions they themselves have created
  • Removed Django admin from /api/admin route
  • Setup custom routes in /api/admin to start separating all endpoints (with the purpose of allowing for more intuitive endpoint-names, further work will be done to solve ABA-218)

Smaller less noteworthy stuff

  • The error messages in the frontend are shit, but to update it the api implementation in the frontend has to be improved and I don't want to do it in this PR
  • Removed the frontend code for slugs and admin-groups as it was a bit premature to add it in my last PR when the functionality is not implemented. Will be added later.

tl:dr; not perfect - but it works and I'd rather iterate on it over time

Visual preview

This is the backend of #857, with only minor changes to the frontend.

Fixes ABA-217, #6, #7

@norbye norbye added the new-feature Pull requests that introduce or issues that suggest a new feature label Mar 1, 2023
@linear
Copy link
Copy Markdown

linear bot commented Mar 1, 2023

@norbye norbye force-pushed the manage-backend branch 2 times, most recently from 4ba49ea to b700e1c Compare April 9, 2023 15:11
@norbye norbye marked this pull request as ready for review April 9, 2023 15:16
Copy link
Copy Markdown
Member

@LudvigHz LudvigHz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good 💯
Not much to complain about :) :shipit:

console.log(JSON.stringify(values, null, 2));
setReturnedData(undefined);
const processedValues = { ...values };
// Convert from iso to iso to re-add timezone data that input[type=datetime-local] removes
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you just not use this input type? There has to be an easy way to use timezone-aware date inputs?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried to find an alternative - but I had some trouble finding something good.. As far as I can tell there aren't really any good alternatives in native HTML5, so if not this then we'd have to add another dependency to sort it out

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah okay. Guess that's fine. But then it should be clear what timezone they are inputting here right? Or will luxon add the timezone stuff? (I guess also the comment isn't super clear 🙃 )

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, I'll think of something to improve the comment. The way it works now is that Luxon takes the local timezone as default when running fromISO() and a timezone is not passed (as datetime-local does), and returns the ISO string including the timezon

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated it now to explicitly type out the default option variables that matter to make it a bit more readable what actually happens

@norbye norbye added the approved Pull requests that have been approved label Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Pull requests that have been approved new-feature Pull requests that introduce or issues that suggest a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants