Bugfix urls namespace and add reverse to missing usages in tests#428
Bugfix urls namespace and add reverse to missing usages in tests#428
Conversation
rwrzesien
requested review from
Karrp,
MartynaAnnaGottschling,
Szymiks,
dybi,
kbeker and
maciejSamerdak
| "next_month_url": "/reports/project/1/2019/4/", | ||
| "recent_month_url": f"/reports/project/1/{current_date.year}/{current_date.month}/", | ||
| "previous_month_url": "/reports/project/1/2019/2/", | ||
| "next_month_url": reverse("project-report-list", args=(self.mixin.kwargs["pk"], 2019, 4)), |
There was a problem hiding this comment.
I see that sometimes we use args and sometimes kwargs in url building. I would suggest that we don't mix them up. My personal choice would be kwargs (greater readability). @rwrzesien, what do you think?
There was a problem hiding this comment.
I would also prefer kwargs because we can see all keys and values associated with them. If you agree @rwrzesien I can change it for you and merge today :)
There was a problem hiding this comment.
Yes, I prefer kwargs too, I just do it in little rush yesterday, go ahead and fix it if you want :)
There was a problem hiding this comment.
I have updated it.
| "next_month_url": "/reports/project/1/2019/4/", | ||
| "recent_month_url": f"/reports/project/1/{current_date.year}/{current_date.month}/", | ||
| "previous_month_url": "/reports/project/1/2019/2/", | ||
| "next_month_url": reverse("project-report-list", args=(self.mixin.kwargs["pk"], 2019, 4)), |
There was a problem hiding this comment.
I would also prefer kwargs because we can see all keys and values associated with them. If you agree @rwrzesien I can change it for you and merge today :)
rwrzesien
force-pushed
the
bugfix-fix-urls-namespace
branch
2 times, most recently
from
1d01317 to
653e348
Compare
kbeker
force-pushed
the
bugfix-fix-urls-namespace
branch
from
653e348 to
45e00d2
Compare
kbeker
changed the title
3 participants
While cleaning up I have seen that we are still using the same url namespace for two Django apps. This should be avoided if not needed, so I have added the fix. Due to this I have discovered there were some places in which we were using hardcoded urls, so I have switched them to use
reverse.