Fixed redirect to cycle with data #1528
Conversation
There was a problem hiding this comment.
@patphongs, great work on this! I read the documentation and played with a couple options but I think this is the best way I can think of implementing this. I'll test thoroughly because I know this impacts multiple parts of the application.
|
@patphongs I've tested locally but not exhaustively. |
|
@ccostino Could use an extra pair of eyes 👀 on this PR too as it might have wide ranging effects on the committee portion of our site. Just to help double check. Thanks! |
There was a problem hiding this comment.
Thank you, @patphongs! All looks good to me here and working fine after testing!
| # ) | ||
| if financials['reports']: | ||
| return redirect( | ||
| reverse('committee-by-id', kwargs={'committee_id': committee['committee_id']}) + '?cycle=' + str(c) |
There was a problem hiding this comment.
I thought that Django had provided a way to add query string parameters to URLs, but apparently not as this ticket is still open. :-)
| @@ -8,7 +8,7 @@ | |||
| url(r'^data/search/$', views.search), | |||
| url(r'^data/advanced/$', views.advanced), | |||
| url(r'^data/candidate/(?P<candidate_id>\w+)/$', views.candidate), | |||
| url(r'^data/committee/(?P<committee_id>\w+)/$', views.committee), | |||
| url(r'^data/committee/(?P<committee_id>\w+)/$', views.committee, name='committee-by-id'), | |||
There was a problem hiding this comment.
Note that with this extra name argument any template making use of this URL can reference it via the name now, too! Here's an example in the Django documentation in case that helps. :-)
There was a problem hiding this comment.
Thanks @ccostino ! It would be nice to add a name for all of them, I think it just helps to understand semantically what each pattern does.
Summary
url_forfunction to construct the redirect, which is using a flask function. To fix this, we needed to convert the redirect to use a django url constructor. Django'sreverse()was used in the PR.Impacted areas of the application
List general components of the application that this PR will affect: