-
-
Notifications
You must be signed in to change notification settings - Fork 123
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
Fix #91: Add labels/badges on issues. #266
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jajodiaraghav -- Looks great! I have left a few comments. Thanks!
website/models.py
Outdated
(4, 'Security'), | ||
(5, 'Typo'), | ||
(6, 'Design'), | ||
(0, 'Unspecified') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about changing Unspecified to General ? I think, that will cover some of the bugs which doesn't comes under the predefined category.
<option value="4">Security</option> | ||
<option value="5">Typo</option> | ||
<option value="6">Design</option> | ||
</select> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be rendered from the backend instead of hard coding the properties here. It'll guarantee a safe submission of our attribute integrity on both the sides.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I have rendered the form element in the edit page from the backend, I couldn't do it in base.html. I am new to Django and would love to see how to do this. Could you please change this part on top of my commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jajodiaraghav -- What issue are you facing while using it in base.html ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{{form.label}} does not render anything in base.html. I did it in a way similar to the one in issue_edit.html. It took me a lot of time trying to debug, but all in vain. Can we merge this and fix the base.html in a different commit. I want to work on issue_edit.html page but couldn't, as it would result in conflicts unless this gets merged.
Or else, please help me fix this.
Thanks :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a look at website/forms.py
(IssueEditForm). The current implementation of the bug submission form needs a complete redesign. Maybe, we can do it completely on another PR.
website/templates/issue_edit.html
Outdated
<option value="5">Typo</option> | ||
<option value="6">Design</option> | ||
</select> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto as the earlier comment.
@@ -6,7 +6,7 @@ class IssueEditForm(forms.ModelForm): | |||
|
|||
class Meta: | |||
model = Issue | |||
fields = ['description', 'screenshot'] | |||
fields = ['description', 'screenshot', 'label'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a required element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't get this part. Could be a bit more explicit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can set the attributes of the form in the backend. So, if we declare a required clause within the form definition in the backend -- It'll make sure that it is required when it is rendered and thus form submission is declined if it is not filled. It also solves the problem that you asked on Slack.
https://docs.djangoproject.com/en/1.11/ref/forms/fields/#django.forms.Field.required
Fix #91