TM-6123, TM-6126, Re-Interate Projected Vacancy #1017
Conversation
M00staff
requested review from
RamelHenderson,
elizabeth-jimenez,
scott062,
jlam-mpc and
cagbakwu
| **response, | ||
| "combinedppgrade": combine_pp_grade(response.pospayplancode, response.posgradecode), | ||
| } | ||
| return response |
There was a problem hiding this comment.
Could we revert this change? I added a combined pp grade field similarly to other list endpoints that display a joint PP and grade field.
There was a problem hiding this comment.
It's still returning all of the base response, just adding a new field on top of that
There was a problem hiding this comment.
spoke with moose about it, it was cause it was erroring cause of the dot notation - pulling with a get should fix 💪
| @@ -9,7 +9,7 @@ | |||
| url(r'^filters/$', views.FSBidAdminProjectedVacancyFiltersView.as_view(), name="admin-projected-vacancies-filters"), | |||
| url(r'^language_offset_options/$', views.FSBidAdminProjectedVacancyLangOffsetOptionsView.as_view(), name="admin-projected-vacancies-lang-offset-options"), | |||
| url(r'^$', views.FSBidAdminProjectedVacancyListView.as_view(), name="admin-projected-vacancies"), | |||
| url(r'^edit/$', views.FSBidAdminProjectedVacancyActionsView.as_view(), name='admin-projected-vacancies-actions'), | |||
| url(r'^edit/$', views.FSBidAdminProjectedVacancyActionsView2.as_view(), name='admin-projected-vacancies-actions'), | |||
There was a problem hiding this comment.
Is there a need to rename all of these functions with a 2 at the end? I don't find it necessary
There was a problem hiding this comment.
its how i originally tested it all without deleting or changing the ones i was gonna replace
| ''' | ||
| Edit Admin Projected Vacancy | ||
| ''' | ||
| fvseqnum = query.get("data", {}) |
There was a problem hiding this comment.
Should the default be None or "" instead of {}? it doesn't quite matter right because {} in the uri string shouldn't throw any warnings?
There was a problem hiding this comment.
yea - i think either way it would fail the same
i just ripped it from the edit panel meeting stuff
but, now i see why its not apples to apples
Co-authored-by: elizabeth-jimenez <elizabeth.jimenez.cs@gmail.com>
Dual Merge:
Ticket
Ticket