From 89ee19778a93a4fea6ba5608ae6d52a241d66a15 Mon Sep 17 00:00:00 2001 From: George Hampton Date: Sat, 21 Oct 2023 13:00:14 +1300 Subject: [PATCH] Meet style requirements Update to previous commit to comply with style requirements --- codewof/programming/filters.py | 2 +- codewof/programming/forms.py | 167 +++++++++++++++++++++++++-------- codewof/programming/models.py | 9 +- codewof/programming/views.py | 116 ++++++++++++++--------- 4 files changed, 206 insertions(+), 88 deletions(-) diff --git a/codewof/programming/filters.py b/codewof/programming/filters.py index abb09a0bd..3d215523a 100644 --- a/codewof/programming/filters.py +++ b/codewof/programming/filters.py @@ -46,7 +46,7 @@ class Meta: class DraftFilter(django_filters.FilterSet): - """Filter for drafts extends FilterSet. Allows for filtering identical to questions""" + """Filter for drafts extends FilterSet. Allows for filtering identical to questions.""" difficulty_level = django_filters.filters.ModelMultipleChoiceFilter( queryset=DifficultyLevel.objects.order_by('level'), diff --git a/codewof/programming/forms.py b/codewof/programming/forms.py index 84d6aeeea..b4616ec41 100644 --- a/codewof/programming/forms.py +++ b/codewof/programming/forms.py @@ -1,9 +1,9 @@ -""" Forms for programming pages """ +"""Forms for programming pages.""" from django import forms from django.urls import reverse from crispy_forms.helper import FormHelper -from crispy_forms.layout import Layout, Submit, Button, Div, HTML, Field +from crispy_forms.layout import Layout, Submit, Button, Div, HTML from crispy_forms.bootstrap import Modal from programming.models import Draft, DifficultyLevel @@ -25,22 +25,22 @@ ] CONCEPTS = { "root": [ - ('display-text','Display Text'), - ('functions','Functions'), - ('inputs','Inputs'), - ('conditionals','Conditionals'), - ('loops','Loops'), - ('string-operations','String Operations'), - ('lists','Lists'), + ('display-text', 'Display Text'), + ('functions', 'Functions'), + ('inputs', 'Inputs'), + ('conditionals', 'Conditionals'), + ('loops', 'Loops'), + ('string-operations', 'String Operations'), + ('lists', 'Lists'), ], "conditionals": [ - ('single-condition','Single Condition'), - ('multiple-conditions','Multiple Conditions'), - ('advanced-conditionals','Advanced Conditionals'), + ('single-condition', 'Single Condition'), + ('multiple-conditions', 'Multiple Conditions'), + ('advanced-conditionals', 'Advanced Conditionals'), ], "loops": [ - ('conditional-loops','Conditional Loops'), - ('range-loops','Range Loops'), + ('conditional-loops', 'Conditional Loops'), + ('range-loops', 'Range Loops'), ], } CONTEXTS = { @@ -58,10 +58,13 @@ ], } + class MacroForm(forms.Form): - """Form for creating/editing macros for new questions""" + """Form for creating/editing macros for new questions.""" + name = forms.CharField(required=True, max_length='20') - possible_values = forms.CharField(widget=forms.Textarea, required=True, help_text='Separate values with a comma (i.e. 6,7). You can escape commas with a backslash (i.e. 6\,7)') + possible_values = forms.CharField(widget=forms.Textarea, required=True, + help_text='Separate values with a comma (i.e. 6,7). You can escape commas with a backslash (i.e. 6\\,7)') def __init__(self, *args, **kwargs): """Add crispyform helper to form.""" @@ -92,8 +95,10 @@ def __init__(self, *args, **kwargs): ), ) + class TestCaseForm(forms.Form): - """Form for creating/editing test cases for new questions""" + """Form for creating/editing test cases for new questions.""" + testcase_type = forms.ChoiceField(required=True, label='Type', choices=TEST_CASE_TYPES) testcase_code = forms.CharField(widget=forms.Textarea, label='Input Code', required=True) @@ -106,7 +111,11 @@ def __init__(self, *args, **kwargs): Modal( 'testcase_type', 'testcase_code', - HTML('Expected output is generated from your provided solution code.'), + HTML( + '' + \ + 'Expected output is generated from your provided solution code.' + \ + '' + ), Button( "close_modal", "Cancel", @@ -127,29 +136,107 @@ def __init__(self, *args, **kwargs): ), ) + class NewQuestionForm(forms.ModelForm): """Form for creating or editing new questions.""" - macros = forms.CharField(widget=forms.Textarea, required=False) - initial_code = forms.CharField(widget=forms.Textarea, required=False) - read_only_lines_top = forms.IntegerField(required=False, help_text="The number of lines at the top of the initial code to make read-only") - read_only_lines_bottom = forms.IntegerField(required=False, help_text="The number of lines at the bottom of the initial code to make read-only") - lines = forms.CharField(widget=forms.Textarea, required=False, label="Extra lines", help_text="Lines to mix in with solution lines") - test_cases = forms.CharField(widget=forms.Textarea, required=False, help_text="Drag test cases up and down to reorder them (may not work with touch devices)") - concepts = forms.MultipleChoiceField(required=False, choices=CONCEPTS["root"], widget=forms.CheckboxSelectMultiple(), label=False) - concept_conditionals = forms.ChoiceField(required=False, choices=CONCEPTS["conditionals"], widget=forms.RadioSelect, label=False) - concept_loops = forms.ChoiceField(required=False, choices=CONCEPTS["loops"], widget=forms.RadioSelect, label=False) + # Fields specific to a type of question + initial_code = forms.CharField( + widget=forms.Textarea, + required=False + ) + read_only_lines_top = forms.IntegerField( + required=False, + help_text="The number of lines at the top of the initial code to make read-only" + ) + read_only_lines_bottom = forms.IntegerField( + required=False, + help_text="The number of lines at the bottom of the initial code to make read-only" + ) + lines = forms.CharField( + widget=forms.Textarea, + required=False, + label="Extra lines", + help_text="Lines to mix in with solution lines" + ) + + # Many-to-many fields + macros = forms.CharField( + widget=forms.Textarea, + required=False + ) + test_cases = forms.CharField( + widget=forms.Textarea, + required=False, + help_text="Drag test cases up and down to reorder them (may not work with touch devices)" + ) + concepts = forms.MultipleChoiceField( + required=False, + choices=CONCEPTS["root"], + widget=forms.CheckboxSelectMultiple(), + label=False + ) + contexts = forms.MultipleChoiceField( + required=False, + choices=CONTEXTS["root"], + widget=forms.CheckboxSelectMultiple(), + label=False + ) + + # Helpers for concepts/contexts + concept_conditionals = forms.ChoiceField( + required=False, + choices=CONCEPTS["conditionals"], + widget=forms.RadioSelect, + label=False + ) + concept_loops = forms.ChoiceField( + required=False, + choices=CONCEPTS["loops"], + widget=forms.RadioSelect, + label=False + ) - contexts = forms.MultipleChoiceField(required=False, choices=CONTEXTS["root"], widget=forms.CheckboxSelectMultiple(), label=False) - context_has_geometry = forms.BooleanField(required=False, initial=False, label="Geometry") - context_mathematics = forms.ChoiceField(required=False, choices=CONTEXTS["mathematics"], widget=forms.RadioSelect, label=False) - context_geometry = forms.ChoiceField(required=False, choices=CONTEXTS["geometry"], widget=forms.RadioSelect, label=False) + context_has_geometry = forms.BooleanField( + required=False, + initial=False, + label="Geometry" + ) + context_mathematics = forms.ChoiceField( + required=False, + choices=CONTEXTS["mathematics"], + widget=forms.RadioSelect, + label=False + ) + context_geometry = forms.ChoiceField( + required=False, + choices=CONTEXTS["geometry"], + widget=forms.RadioSelect, + label=False + ) - difficulty_level = forms.ChoiceField(required=False, choices=QUESTION_DIFFICULTY_CHOICES, initial=QUESTION_DIFFICULTY_CHOICES[0], label='Difficulty') + # Defining a custom widget for difficulty to enable correct display names for the options + difficulty_level = forms.ChoiceField( + required=False, + choices=QUESTION_DIFFICULTY_CHOICES, + initial=QUESTION_DIFFICULTY_CHOICES[0], + label='Difficulty' + ) class Meta: + """Defines attributes for crispy form generation.""" model = Draft - fields = ["title", "question_type", "difficulty_level", "question_text", "macros", "solution", "concepts", "contexts", "lines"] + fields = [ + "title", + "question_type", + "difficulty_level", + "question_text", + "macros", + "solution", + "concepts", + "contexts", + "lines" + ] labels = { "title": "Question Title", "question_type": "Type", @@ -159,7 +246,6 @@ class Meta: } - def __init__(self, *args, **kwargs): """Add crispyform helper to form.""" super().__init__(*args, **kwargs) @@ -175,7 +261,7 @@ def __init__(self, *args, **kwargs): Div( Div( HTML('{% include "programming/question_creation/macros_display_table.html" %}'), - css_class = "table-responsive", + css_class="table-responsive", ), Button( "new_macro", @@ -184,7 +270,7 @@ def __init__(self, *args, **kwargs): css_class="btn-outline-secondary", type="button", ), - css_class = "text-center", + css_class="text-center", ), 'solution', HTML('{% include "programming/question_components/indentation-warning.html" %}'), @@ -233,7 +319,7 @@ def __init__(self, *args, **kwargs): Div( HTML('{% include "programming/question_creation/creation_test_case_table.html" %}'), css_class = "table-responsive", - css_id = "test_case_table_container", + css_id="test_case_table_container", ), Button( "new_test_case", @@ -242,7 +328,7 @@ def __init__(self, *args, **kwargs): css_class="btn-outline-secondary", type="button", ), - css_class = "text-center", + css_class="text-center", ), Div( HTML(f'Cancel'), @@ -253,8 +339,9 @@ def __init__(self, *args, **kwargs): def clean(self, *args, **kwargs): """ - Overrides the validation, as difficulty level is a foreign key and would otherwise - raise an error. + Override the values returned from cleaning the form. + + Difficulty level is a foreign key and would otherwise raise an error. """ cleaned_data = self.cleaned_data cleaned_data['difficulty_level'] = DifficultyLevel.objects.get( diff --git a/codewof/programming/models.py b/codewof/programming/models.py index 51253feda..4ac7afeb8 100644 --- a/codewof/programming/models.py +++ b/codewof/programming/models.py @@ -301,6 +301,7 @@ class Meta: verbose_name = 'Draft' verbose_name_plural = 'Drafts' + class DraftTestCase(TranslatableModel): """Base class for a draft for TestCase.""" @@ -323,6 +324,7 @@ def __str__(self): return self.type pass + class DraftMacro(models.Model): """A macro for a draft question.""" @@ -333,8 +335,9 @@ class DraftMacro(models.Model): on_delete=models.CASCADE, ) + class DraftMacroValue(TranslatableModel): - """A potential value for a draft macro to take""" + """A potential value for a draft macro to take.""" macro = models.ForeignKey( DraftMacro, @@ -343,7 +346,9 @@ class DraftMacroValue(TranslatableModel): ) value = models.TextField() + # ----- Base question classes ------------------------------------------------- + class Question(TranslatableModel): """Base class for a question for CodeWOF. @@ -431,7 +436,7 @@ class Macro(models.Model): class MacroValue(TranslatableModel): - """A potential value for a macro to take""" + """A potential value for a macro to take.""" macro = models.ForeignKey( Macro, diff --git a/codewof/programming/views.py b/codewof/programming/views.py index 2e7eaba88..17ff9594a 100644 --- a/codewof/programming/views.py +++ b/codewof/programming/views.py @@ -4,7 +4,7 @@ import yaml import os from django.contrib.auth.decorators import login_required -from django.views import generic, View +from django.views import generic from django.urls import reverse from django.db.models import Count, Max, Exists, OuterRef from django.db.models.functions import Coalesce @@ -99,6 +99,7 @@ def get_context_data(self, **kwargs): context['filter_button_pressed'] = "submit" in self.request.GET return context + class DraftQuestionListView(LoginRequiredMixin, FilterView): """View for listing draft questions created by the user.""" @@ -131,19 +132,21 @@ def get_context_data(self, **kwargs): Returns: Dictionary of context data. """ - user = self.request.user context = super().get_context_data(**kwargs) context['filter_formatter'] = create_filter_helper("programming:draft_list") context['filter_button_pressed'] = "submit" in self.request.GET return context + class DeleteQuestionView(LoginRequiredMixin, generic.base.TemplateView): - """ A "view" to handle requests to delete drafts """ + """A "view" to handle requests to delete drafts.""" + model = Draft def get_object(self, **kwargs): - """Get draft object""" + """Get draft object.""" + try: draft = Draft.objects.get_subclass( pk=self.kwargs['pk'] @@ -154,6 +157,8 @@ def get_object(self, **kwargs): return draft def post(self, request, *args, **kwargs): + """Check that the user is the author of the draft, and if so delete it.""" + self.object = self.get_object() if not request.user.is_authenticated or not request.user == self.object.author.user: return HttpResponseForbidden() @@ -161,8 +166,10 @@ def post(self, request, *args, **kwargs): messages.info(request, 'Question deleted successfully.') return redirect(reverse('programming:draft_list')) + class SubmitQuestionView(LoginRequiredMixin, generic.CreateView): - """ Handles request to create a new question from a draft """ + """Handles request to create a new question from a draft.""" + template_name = 'programming/add_question.html' model = Draft @@ -178,7 +185,8 @@ def get_object(self, **kwargs): return draft def create_question_from_draft(self): - """Create a question model based on draft WITHOUT saving it""" + """Create a question model based on draft without saving it.""" + # Choose question model appropriately for the type model_name = f"QuestionType{self.object.question_type.title()}" class_ = getattr(programming.models, model_name) @@ -186,28 +194,36 @@ def create_question_from_draft(self): # Populate model with appropriate information # Default fields - question.slug=self.object.slug - question.languages=self.object.languages - question.title=self.object.title - question.question_type=self.object.question_type - question.question_text=self.object.question_text - question.solution=self.object.solution - question.difficulty_level=self.object.difficulty_level + question.slug = self.object.slug + question.languages = self.object.languages + question.title = self.object.title + question.question_type = self.object.question_type + question.question_text = self.object.question_text + question.solution = self.object.solution + question.difficulty_level = self.object.difficulty_level # Model-specific fields if self.object.question_type == 'parsons': # Parsons - question.lines=self.object.lines + question.lines = self.object.lines elif self.object.question_type == 'debugging': # Debugging - question.initial_code=self.object.initial_code - question.read_only_lines_top=self.object.read_only_lines_top - question.read_only_lines_bottom=self.object.read_only_lines_bottom + question.initial_code = self.object.initial_code + question.read_only_lines_top = self.object.read_only_lines_top + question.read_only_lines_bottom = self.object.read_only_lines_bottom return question def is_valid_question(self, request): + """ + Check whether the question created is valid. + + Creates a question from the draft in self.object, adds messages for + any problems to the request, and returns a boolean describing whether + the question is valid. + """ + valid = True # Create (do NOT save) a question model based on draft question = self.create_question_from_draft() @@ -248,9 +264,11 @@ def is_valid_question(self, request): # Return boolean of whether it is valid return valid - def generate_YAML(self): + def generate_yaml(self): """ - Generates the YAML to go in questions.yaml with the following format: + Generate the YAML to go in questions.yaml. + + Generates yaml with the following format: title: types: - @@ -266,16 +284,17 @@ def generate_YAML(self): [contexts: - ] """ + # Preparation of different question types before_test_cases = [ - {'type' : [self.object.question_type]}, + {'type': [self.object.question_type]}, ] if self.object.question_type == 'parsons': - before_test_cases[0]['types'] = ['function', 'parsons'] + before_test_cases[0]['types'] = ['function', 'parsons'] del before_test_cases[0]['type'] if self.object.lines is not None: - before_test_cases.append({'parsons-extra-lines' : [self.object.extra_lines]}) + before_test_cases.append({'parsons-extra-lines': [self.object.extra_lines]}) elif self.object.question_type == 'debugging': before_test_cases += [ {'number_of_read_only_lines_top': self.object.number_of_read_only_lines_top}, @@ -285,8 +304,8 @@ def generate_YAML(self): # Test cases test_cases = [] for test_case in list(self.object.draft_test_cases.values()): - test_cases.append({test_case['number'] : test_case['type']}) - test_cases = [{'test-cases' : sorted(test_cases, key=lambda x: x.keys())}] + test_cases.append({test_case['number']: test_case['type']}) + test_cases = [{'test-cases': sorted(test_cases, key=lambda x: x.keys())}] # Difficulty, concepts, and contexts after_test_cases = [ @@ -298,10 +317,10 @@ def generate_YAML(self): after_test_cases.append({'contexts': [context['slug'] for context in contexts]}) # Join together - return [{self.object.title : before_test_cases + test_cases + after_test_cases}] + return [{self.object.title: before_test_cases + test_cases + after_test_cases}] def generate_markdown(self): - """For question.md file""" + """Create markdown for question.md file.""" lines = [f"# {self.object.title}"] for line in self.object.question_text.split('

'): @@ -309,13 +328,13 @@ def generate_markdown(self): .replace('', '**') .replace('', '**') .replace('', '`') - .replace('', '`') - ) + .replace('', '`')) return '\n'.join(lines) def generate_files(self): - """Generates all the stored files """ + """Generate all the stored files for a question.""" + # YAML for structure file with open(f'./programming/review/structure/{self.object.slug}.yaml', 'w') as file: yaml.dump(self.generate_YAML(), file) @@ -336,9 +355,10 @@ def generate_files(self): # Iterate through test cases test_case_suffix = 'input' if self.object.question_type == 'program' else 'code' for test_case in list(self.object.draft_test_cases.values()): - with open(f'./programming/review/en/{self.object.slug}/test-case-{test_case["number"]}-{test_case_suffix}.txt', 'w') as file: + test_case_file_prefix = f'./programming/review/en/{self.object.slug}/test-case-{test_case["number"]}' + with open(f'{test_case_file_prefix}-{test_case_suffix}.txt', 'w') as file: file.write(test_case["test_code"]) - with open(f'./programming/review/en/{self.object.slug}/test-case-{test_case["number"]}-output.txt', 'w') as file: + with open(f'{test_case_file_prefix}-output.txt', 'w') as file: file.write(test_case["expected_output"]) # Create YAML file to store the macros @@ -353,6 +373,8 @@ def generate_files(self): yaml.dump(macros, file) def post(self, request, *args, **kwargs): + """Handle post request to submit a draft.""" + if not request.user.is_authenticated: return HttpResponseForbidden() @@ -370,12 +392,11 @@ def post(self, request, *args, **kwargs): return redirect(reverse('programming:draft_list')) # Question was invalid, send user to question creation form to fix the issues - return redirect(reverse('programming:edit_draft', kwargs={'pk':self.kwargs['pk']})) + return redirect(reverse('programming:edit_draft', kwargs={'pk': self.kwargs['pk']})) + class DraftQuestionView(LoginRequiredMixin, generic.CreateView, generic.UpdateView): - """ - Displays the form for editing a draft question. - """ + """Display the form for editing a draft question.""" template_name = 'programming/add_question.html' template_name_suffix = '' @@ -398,15 +419,15 @@ def get_object(self, **kwargs): return draft def get_context_data(self, **kwargs): - """Provide the context data for the create/edit question view. + """ + Provide the context data for the create/edit question view. Returns: Dictionary of context data. """ - user = self.request.user context = super().get_context_data(**kwargs) context['question'] = self.object - if context['question'] != None: + if context['question'] is not None: test_cases = self.object.draft_test_cases.values() context['test_cases'] = test_cases context['test_cases_json'] = json.dumps(list(test_cases)) @@ -422,7 +443,7 @@ def get_context_data(self, **kwargs): context['macros_json'] = json.dumps(list(macros)) context['forms'] = { - "main_form": NewQuestionForm(instance = context['question']), + "main_form": NewQuestionForm(instance=context['question']), "macro_form": MacroForm(), "test_case_form": TestCaseForm(), } @@ -430,7 +451,8 @@ def get_context_data(self, **kwargs): return context def _custom_split(self, string): - """Splits on commas, but allows backslashes to escape splitting""" + """Split on commas, but allow backslashes to escape splitting.""" + parts = string.split(',') i = 1 output = [parts[0]] @@ -445,13 +467,15 @@ def _custom_split(self, string): return output def form_valid(self, form, *args, **kwargs): + """Save the draft when a valid form is submitted.""" + if not self.request.user.is_authenticated: return HttpResponseForbidden() self.object = self.get_object() # First save the draft draft = form.save(commit=False) - if self.object == None: + if self.object is None: # New draft draft.languages = ['en'] draft.author_id = self.request.user.id @@ -556,18 +580,20 @@ def form_valid(self, form, *args, **kwargs): def _generate_slug(self, cleaned): """ - Creates a slug for new questions in the same style as existing questions: - - Make title lowercase - - Replace spaces with hyphens - - Add - to the end + Create a slug for new questions in a similar style to existing questions. + + Make title lowercase, replace spaces with hyphens, and add - to the end. """ return f"{cleaned['title'].lower().replace(' ', '-')}-{cleaned['question_type']}" def form_invalid(self, form): """Take action if form is invalid.""" + return super().form_invalid(form) def get_success_url(self): + """Define the url to visit on a success.""" + return reverse('programming:draft_list')