search

tulsawebdevs / django-multi-gtfs

Django app to import and export General Transit Feed Specification (GTFS)
http://tulsawebdevs.org/
Apache License 2.0
50 stars 32 forks source link

Fix seconds conversion bug plus minor model changes #44

Closed alaw005 closed 6 years ago

alaw005 commented 9 years ago

This was supposed to only include the seconds conversion bug fix, but I have left other two model changes in pull request as I would like to suggest these changes and I can't work out how to remove.

The seconds conversion bug showed itself on the admin interface when I was trying to add a new entry to the Frequency model. I got an "invalid literal for int() with base 10: ''. The traceback is below (sorry couldn't work out how to paste so looks nice).

FeedInfo - I have changed to have a one-to-one relationship to Feed. This is because each Feed should only have one FeedInfo record. This ensures that the user cannot add an additional FeedInfo record in the admin interface.

Trip - I have added additional step that updates geometry on save so that if the user changes the shape_id the geometry is updated to match new shape.

I don't know how to run tests yet so haven't tried.

Environment:

Request Method: GET Request URL: http://192.168.15.216:8000/admin/multigtfs/frequency/add/

Django Version: 1.6.11 Python Version: 2.7.6 Installed Applications: ['django.contrib.admin', 'django.contrib.auth', 'django.contrib.contenttypes', 'django.contrib.sessions', 'django.contrib.messages', 'django.contrib.staticfiles', 'django.contrib.gis', 'exploreapp', 'south', 'multigtfs'] Installed Middleware: ('django.contrib.sessions.middleware.SessionMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware')

Traceback: File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/core/handlers/base.py" in get_response

  1. response = wrapped_callback(request, _callback_args, *_callback_kwargs) File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/contrib/admin/options.py" in wrapper
  2. return self.admin_site.admin_view(view)(_args, *_kwargs) File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/utils/decorators.py" in _wrapped_view
  3. response = view_func(request, _args, *_kwargs) File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  4. response = view_func(request, _args, *_kwargs) File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/contrib/admin/sites.py" in inner
  5. return view(request, _args, *_kwargs) File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/utils/decorators.py" in _wrapper
  6. return bound_func(_args, *_kwargs) File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/utils/decorators.py" in _wrapped_view
  7. response = view_func(request, _args, *_kwargs) File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/utils/decorators.py" in bound_func
  8. return func(self, _args2, *_kwargs2) File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/db/transaction.py" in inner
  9. return func(_args, *_kwargs) File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/contrib/admin/options.py" in add_view
  10. form = ModelForm(initial=initial) File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/forms/models.py" in init
  11. self.instance = opts.model() File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/db/models/base.py" in init
  12. setattr(self, field.attname, val) File "/vagrant/project/dev/django-multi-gtfs/myenv/lib/python2.7/site-packages/django/db/models/fields/subclassing.py" in set
  13. obj.dict[self.field.name] = self.field.to_python(value) File "/vagrant/project/dev/django-multi-gtfs/multigtfs/models/fields/seconds.py" in to_python
  14. seconds = int(svalue)

Exception Type: ValueError at /admin/multigtfs/frequency/add/ Exception Value: invalid literal for int() with base 10: ''

jwhitlock commented 9 years ago

I've got a busy weekend, so it might be a while before I can give detailed feedback. This PR needs work.

These are independent changes, and should be 3 pull requests. Check out http://sethrobertson.github.io/GitFixUm/fixup.html for advice on splitting commits into branches.

Fixing bugs requires tests, which implies that you can run the tests locally. make test and make qa are good starting points, to run locally what TravisCI runs on check-in. ./manage.py test will give more control, such as running just a single test or suite of tests. See CONTRIBUTORS.rst for some more advice.

alaw005 commented 9 years ago

Wow, that was my first test and successful ... almost worth staying up all night for! I can see the value of testing as my earlier fix broke something else and with the tests so easy to see why/how. I have also removed the other changes to include as separate pull requests.

jwhitlock commented 6 years ago

It appears the remaining issue is setting a SecondsField to an empty string is expected to set it to 0 seconds.

I have to admit, I haven't spent a lot of time attempting to manupulate data through the admin. I'm mostly been importing and exporting feeds. I tried adding a Frequency, and it would not allow me to set start_time or end_time to blank string (I got a "This field is required") error. The choices for exact_times are incorrect. The choices at 0 or 1, but the field is a CharField (with blank=True to allow an empty string), so they should be '0' and '1'.

I think the remaining code in this pull request reflects the code from three years ago, and the source branch is lost to GitHub, so I'm closing the PR.