Migrated Events app to CBV #364

amruthasangeeth · 2016-07-29T17:17:52Z

Migrated function based views in Events app to class based views. The following are the views used:
ListView - To list the Events
UpdateView - To edit the Event details
DeleteView - To delete a Event object
#288.

tapaswenipathak · 2016-07-30T16:20:10Z

@amruthasangeeth Travis is throwing errors, can you please fix them?

AIL: test_edit_event_with_invalid_job_date (administrator.tests.test_settings.Settings)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/systers/vms/vms/administrator/tests/test_settings.py", line 689, in test_edit_event_with_invalid_job_date
    self.settings_page)
AssertionError: u'http://localhost:8081/event/list/' == u'http://localhost:8081/event/list/'

This failure is present in your other pull requests as well, @smarshy found out that the value should not be same to pass this assertion.

Let us know if you are not able to resolve the errors.

cc @nida @nidhimj22.

amruthasangeeth · 2016-08-01T19:43:00Z

@smarshy Can you spot the error it is showing in test? I am not able to find it.

smarshy · 2016-08-02T02:49:34Z

@amruthasangeeth On editing event, it isn't getting saved. Also, you are having same value retention problem as in the jobs pr. What that means is that if the form gets refreshed / reloaded for some reason ( say invalid values entered) the values that the user has already entered shouldn't vanish.

smarshy · 2016-08-02T02:52:21Z

vms/event/models.py

+            ),
+        ],
+        blank=True,
+        null=True,


@amruthasangeeth Are there any changes in this model?
I can spot only the autoincrement one

@smarshy Only the id is the new one.

@amruthasangeeth Complete file is coming as diff, it is really difficult to review. Can you please undo the unnecessary changes (cleanups, spacing)?

tapaswenipathak · 2016-08-06T11:11:50Z

@amruthasangeeth Please fix the errors in this and all the consecutive pull requests.

smarshy · 2016-08-09T04:45:31Z

@amruthasangeeth This has less errors - both are related to edit event. Please try testing this locally. Edit an event with valid details and check if it gets saved.

Also, the second error is due to this

Here, since the end date is not wrong, it isn't expected to be erased. Since the same error did not take place for create event, I think this too is related to edit event

coveralls · 2016-08-10T19:39:59Z

Coverage increased (+0.4%) to 84.3% when pulling b1d01d01d8dfc280b947f03923b251df8dccfc7b on amruthasangeeth:ShiftEventsToCBV into 91677ce on systers:develop.

amruthasangeeth · 2016-08-10T19:42:02Z

@tapasweni-pathak All the test cases here are passed now :)

tapaswenipathak · 2016-08-11T03:52:00Z

vms/event/forms.py

+
+class EventDateForm(forms.Form):
+    start_date = forms.DateField(required=True)
+    end_date = forms.DateField(required=True)


@amruthasangeeth What has been changed in this file? Nothing, right? Did you run pep8?

@tapasweni-pathak No changes are made in this file.

tapaswenipathak · 2016-08-11T03:59:10Z

@amruthasangeeth Great. :)

If you see you will find that complete files are coming in the diff, it becomes very difficult to review such a diff. Can you please undo the unnecessary changes and only have the useful ones in this pull request?

You can submit a new pull request for the cleanups, spacing issues e.t.c.

tapaswenipathak · 2016-08-12T11:42:42Z

vms/event/templates/event/delete.html

@@ -4,7 +4,7 @@

 {% block setting_content %}
    <div class="spacer"></div>
-    <form class="form-horizontal" action="{% url 'event:delete' event_id %}" method="post">
+    <form class="form-horizontal" action="" method="post">


@amruthasangeeth Where are we redirecting to delete?

@tapasweni-pathak DeleteView works just like that.

@amruthasangeeth Then do we need this form tag ?

Migrated function based views in Events app to class based views

amruthasangeeth · 2016-08-18T14:17:42Z

vms/event/views.py

+    template_name = "event/list.html"
+
+    def get_queryset(self):
+        events = Event.objects.all().order_by('name')


@tapasweni-pathak Updated the PR by making the list in sorted order.

@amruthasangeeth Cool.

coveralls · 2016-08-18T14:26:31Z

Coverage increased (+0.4%) to 83.693% when pulling f4bf510 on amruthasangeeth:ShiftEventsToCBV into 5dadcd6 on systers:develop.

smarshy · 2016-08-19T05:11:30Z

@tapasweni-pathak There are tests for this, they are commented out as the current vms does not redirect to no admin pages uniformly. I have added a test for this - here
#325 is related to this.
Once all cases are covered, I can uncomment it and check. Or should I do it now itself? The build might fail.

tapaswenipathak · 2016-08-23T06:25:27Z

@smarshy Okay. Do we have these tests for the cases when it is being redirected to no admin page or all are commented?

amruthasangeeth force-pushed the ShiftEventsToCBV branch 5 times, most recently from 86f63fe to 0b14da1 Compare August 1, 2016 19:06

smarshy reviewed Aug 2, 2016
View reviewed changes

amruthasangeeth force-pushed the ShiftEventsToCBV branch 2 times, most recently from d7556ca to b1d01d0 Compare August 10, 2016 19:23

tapaswenipathak reviewed Aug 11, 2016
View reviewed changes

tapaswenipathak reviewed Aug 12, 2016
View reviewed changes

tapaswenipathak added the Needs changes label Aug 18, 2016

Migrated Events app to CBV

f4bf510

Migrated function based views in Events app to class based views

amruthasangeeth force-pushed the ShiftEventsToCBV branch from b1d01d0 to f4bf510 Compare August 18, 2016 14:16

amruthasangeeth reviewed Aug 18, 2016
View reviewed changes

tapaswenipathak merged commit 09a6837 into anitab-org:develop Aug 18, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrated Events app to CBV #364

Migrated Events app to CBV #364

amruthasangeeth commented Jul 29, 2016 •

edited by tapaswenipathak

Loading

tapaswenipathak commented Jul 30, 2016 •

edited

Loading

amruthasangeeth commented Aug 1, 2016 •

edited

Loading

smarshy commented Aug 2, 2016

smarshy Aug 2, 2016

amruthasangeeth Aug 6, 2016

tapaswenipathak Aug 11, 2016

tapaswenipathak commented Aug 6, 2016

smarshy commented Aug 9, 2016

coveralls commented Aug 10, 2016

amruthasangeeth commented Aug 10, 2016

tapaswenipathak Aug 11, 2016

amruthasangeeth Aug 11, 2016

tapaswenipathak commented Aug 11, 2016

tapaswenipathak Aug 12, 2016

amruthasangeeth Aug 13, 2016

tapaswenipathak Aug 18, 2016 •

edited

Loading

amruthasangeeth Aug 18, 2016

tapaswenipathak Aug 18, 2016

coveralls commented Aug 18, 2016

smarshy commented Aug 19, 2016

tapaswenipathak commented Aug 23, 2016

Migrated Events app to CBV #364

Migrated Events app to CBV #364

Conversation

amruthasangeeth commented Jul 29, 2016 • edited by tapaswenipathak Loading

tapaswenipathak commented Jul 30, 2016 • edited Loading

amruthasangeeth commented Aug 1, 2016 • edited Loading

smarshy commented Aug 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tapaswenipathak commented Aug 6, 2016

smarshy commented Aug 9, 2016

coveralls commented Aug 10, 2016

amruthasangeeth commented Aug 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tapaswenipathak commented Aug 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tapaswenipathak Aug 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Aug 18, 2016

smarshy commented Aug 19, 2016

tapaswenipathak commented Aug 23, 2016

amruthasangeeth commented Jul 29, 2016 •

edited by tapaswenipathak

Loading

tapaswenipathak commented Jul 30, 2016 •

edited

Loading

amruthasangeeth commented Aug 1, 2016 •

edited

Loading

tapaswenipathak Aug 18, 2016 •

edited

Loading