-
Notifications
You must be signed in to change notification settings - Fork 4
Added description field in events #723
Added description field in events #723
Conversation
Hi @abhishekarya286 @ayushgarg1804 @kamsuri Please review |
vms/event/models.py
Outdated
description = models.TextField( | ||
blank=True, | ||
validators=[ | ||
RegexValidator(r'^[(A-Z)|(a-z)|(0-9)|(\s)|(\.)|(,)|(\-)|(!)]+$', ), |
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.
Does this define all the possibilities for description field?
I think it limits a few rather.
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.
@kamsuri I guess I need to remove these validations, since in portal there are not any.
18b1ac2
to
1d10057
Compare
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.
Also require tests before merge
@@ -44,7 +44,28 @@ | |||
</div> | |||
</div> | |||
{% endif %} | |||
|
|||
{% if form.description.errors %} |
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.
minor: indentation
vms/event/models.py
Outdated
@@ -12,6 +12,8 @@ class Event(models.Model): | |||
r'^[(A-Z)|(a-z)|(0-9)|(\s)|(\.)|(,)|(\-)|(!)|(\')]+$', ), | |||
], | |||
) | |||
description = models.TextField( | |||
blank=True) |
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.
minor: change it to same line.
1d10057
to
5b3052a
Compare
@ayushgarg1804 Done with the changes, please review. |
5b3052a
to
ad9bc90
Compare
Hi @ayushgarg1804 @abhishekarya286 @kamsuri Done with the tests for this PR. Please review. |
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.
Please remove commit 77a6c23 in the PR
13a105f
to
89185c0
Compare
@ayushgarg1804 Done with removing the commit. Please review. |
4a71261
to
322decc
Compare
@anjali-dhanuka UI tests missing, please update. |
@Monal5031 But there are no constraints on description field, it's just a simple TextField so no UI errors are going to appear. I was in dilemna before regarding this so I was waiting for you to complete |
2df4e12
to
422306d
Compare
@anjali-dhanuka You can add a UI test for display of correct description, since we can't add for errors we can skip that test |
@Monal5031 There is no view where the description of an event is shown. |
5c7d6c0
to
c5246e0
Compare
You can create an event with description and view its contents in form edit. That can work as display of event description IMO |
c5246e0
to
2939197
Compare
@Monal5031 Done with the UI tests I added a check in the detail view, please review. Also @abhishekarya286 @kamsuri @nida Please review |
@anjali-dhanuka travis failing, please restart it im on mobile atm |
validations removed formatting implemented
description field tests added list indexed str removed and create_event function changed save issues solved vola api updated with description UI tests for description added
2939197
to
4419e21
Compare
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.
LGTM
Description
Added description field for events
Fixes #722
Type of Change:
Code/Quality Assurance Only
How Has This Been Tested?
Checklist:
Code/Quality Assurance Only