Skip to content
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

Update api and structure to support json schema #1178

Closed
wants to merge 5 commits into from
Closed

Conversation

norbye
Copy link
Member

@norbye norbye commented Jul 24, 2023

tl;dr: these PRs enable admins to customize info text, what questions are asked and how many questions are asked (with no need of Webkom supervision - hopefully)

Chunky changes, so I tried to spread it out a little to see if it makes it simpler to review. In case that is not the case, the complete code is available in branch https://github.com/webkom/admissions/tree/json-schema-complete. It might be necessary to run the code from this branch if you want to test it locally, as the codesplitting between the three PRs might be faulty.

This PR is the main bulk of functionality changes, and is required to make the other PRs run and make sense.

There is still plenty to be desired, so this is in no way perfect - but this should be a decent MVP with all necessary functionality intact, and enough to merge it into staging and continue working on it.

Testing is also non-existent, but I'll get to that later..

Changes

Previously, admissions were hardcoded so every group had a response_label string - which is the text prompt that applying users see over the textarea. Each application and group application then had a stored text string, which was whatever the user wrote into the application.

Now, I have restructured it so that each group contains a questions JSON object, and each application and group application contains a responses JSON object.

The questions-object consists of what the group admins choose for it to be, configured through the admin panel (PR #1180).
So far this can either be a text (normal paragraph), textarea (long text input), textinput (short text) or a phoneinput (norwegian phone number). Each question contains an id, name and label that is used to display the question.

The responses-object consists of whatever was inserted into the application, in a pair of id and value - where the id matches the id from the corresponding question.

Example of the JSON bodies

Questions;

[{
    "type": "textarea",
    "id": "550e8400-e29b-41d4-a716-446655440000",
    "name": "Søknadstekst",
    "label": "Hvorfor har du lyst til å søke denne gruppen? Skriv litt om din motivasjon."
}]

Responses;

{"550e8400-e29b-41d4-a716-446655440000": "Jeg har kjempelyst ..."}

Resolves ABA-513

@norbye norbye added the review-needed Pull requests that need review label Jul 24, 2023
@norbye norbye requested a review from a team July 24, 2023 06:00
@linear
Copy link

linear bot commented Jul 24, 2023

admissions/admissions/views.py Show resolved Hide resolved
frontend/src/index.tsx Show resolved Hide resolved
Comment on lines -208 to +242
text = serializers.SerializerMethodField()
# text = serializers.SerializerMethodField()
user = ShortUserSerializer()

def get_text(self, obj):
is_filtered = getattr(obj, "group_applications_filtered", False)
if is_filtered:
return None
return obj.text
# def get_text(self, obj):
# is_filtered = getattr(obj, "group_applications_filtered", False)
# if is_filtered:
# return None
# return obj.text
Copy link
Member Author

Choose a reason for hiding this comment

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

Whopsies... Will delete this before merging

Copy link
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.

Skimmed through on my phone. Looks good :)

Although this seems like it would be a lot easier to do using pydantic (schema definition, types, validation etc) but this works so I won't force you to do anything. But worth keeping in mind for similar usecases later.

Comment on lines +299 to +300
print(responses)
print(question)
Copy link
Member

Choose a reason for hiding this comment

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

:)

admissions/admissions/serializers.py Show resolved Hide resolved
@norbye
Copy link
Member Author

norbye commented Jul 25, 2023

Although this seems like it would be a lot easier to do using pydantic (schema definition, types, validation etc) but this works so I won't force you to do anything. But worth keeping in mind for similar usecases later.

Wow thanks a lot, that looks like a much better option! Will take a look at it

@norbye norbye marked this pull request as draft August 22, 2023 08:24
@norbye
Copy link
Member Author

norbye commented Aug 22, 2023

Converted to draft to ensure it is not merged before 2023 admissions, will merge some time after that.

@norbye norbye added approved Pull requests that have been approved and removed review-needed Pull requests that need review labels Aug 22, 2023
@norbye norbye marked this pull request as ready for review October 26, 2023 23:30
@norbye norbye changed the base branch from staging to master October 26, 2023 23:31
@norbye
Copy link
Member Author

norbye commented Oct 27, 2023

This was quite annoying to rebase at this point, so I'm closing it off for now - but keeping the branch so I can look into it at a later time, possibly changing the backend to use pydantic(:

@norbye norbye closed this Oct 27, 2023
@norbye norbye deleted the json-schema branch January 8, 2024 22:37
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants