-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: implement the existing alteration list and alteration deletion for the applicant (HL-1154) #2925
Conversation
4326474
to
308815f
Compare
API branch is deployed to platta: https://kesaseteli-pr2925.api.dev.hel.ninja/healthz πππ |
API branch is deployed to platta: https://tet-pr2925.api.dev.hel.ninja/healthz πππ |
EMPLOYER branch is deployed to platta: https://kesaseteli-employer-pr2925.dev.hel.ninja πππ |
ADMIN branch is deployed to platta: https://tet-admin-ui-pr2925.dev.hel.ninja πππ |
APPLICANT branch is deployed to platta: https://helsinkilisa-ui-pr2925.dev.hel.ninja πππ |
API branch is deployed to platta: https://helsinkilisa-pr2925.api.dev.hel.ninja/healthz πππ |
TestCafe result is success for https://tet-admin-ui-pr2925.dev.hel.ninja ππππ |
HANDLER branch is deployed to platta: https://helsinkilisa-ui-handler-pr2925.dev.hel.ninja πππ |
TestCafe result is success for https://kesaseteli-employer-pr2925.dev.hel.ninja ππππ |
TestCafe result is success for https://helsinkilisa-ui-pr2925.dev.hel.ninja ππππ |
TestCafe handler result is failed for https://helsinkilisa-ui-handler-pr2925.dev.hel.ninja πΏπ’π₯π₯ |
HANDLER branch is deployed to platta: https://kesaseteli-handler-pr2925.dev.hel.ninja πππ |
TestCafe result is success for https://kesaseteli-handler-pr2925.dev.hel.ninja ππππ |
YOUTH branch is deployed to platta: https://tet-youth-ui-pr2925.dev.hel.ninja πππ |
TestCafe result is success for https://tet-youth-ui-pr2925.dev.hel.ninja ππππ |
YOUTH branch is deployed to platta: https://kesaseteli-youth-pr2925.dev.hel.ninja πππ |
TestCafe result is success for https://kesaseteli-youth-pr2925.dev.hel.ninja ππππ |
TestCafe handler result is failed for https://helsinkilisa-ui-handler-pr2925.dev.hel.ninja πΏπ’π₯π₯ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than what's commented, LGTM π
if company != alteration.application.company: | ||
raise PermissionDenied(_("You are not allowed to do this action")) | ||
|
||
if request.user.is_handler(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use rest framework's permission_classes = [BFIsHandler]
on the viewset level. Or is the API shared with applicant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry vice versa so BFIsApplicant
in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say if they're shared, or plan to share then decouple as applicant and handler viewset. Otherwise role checking becomes swiss cheese if permission_class
is not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both currently use the same API, yeah. Splitting doesn't sound like a bad idea, so I should do that β I'll do so in the following PRs during the handler side implementation. I'll have to take a closer look at the application list where that already exists but I imagine it should be pretty straightforward.
@@ -48,6 +50,16 @@ const DecisionSummary = ({ application }: Props): JSX.Element => { | |||
alteration.alterationType === ALTERATION_TYPE.TERMINATION | |||
); | |||
|
|||
const sortedAlterations = application.alterations?.sort((a, b) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return new Date(a.endDate) - new Date(b.endDate)
is enough in JS π
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right, that's true. I'll simplify it to that.
API branch is deployed to platta: https://kesaseteli-pr2925.api.dev.hel.ninja/healthz πππ |
API branch is deployed to platta: https://tet-pr2925.api.dev.hel.ninja/healthz πππ |
ADMIN branch is deployed to platta: https://tet-admin-ui-pr2925.dev.hel.ninja πππ |
TestCafe admin result is failed for https://tet-admin-ui-pr2925.dev.hel.ninja πΏπ’π₯π₯ |
EMPLOYER branch is deployed to platta: https://kesaseteli-employer-pr2925.dev.hel.ninja πππ |
APPLICANT branch is deployed to platta: https://helsinkilisa-ui-pr2925.dev.hel.ninja πππ |
API branch is deployed to platta: https://helsinkilisa-pr2925.api.dev.hel.ninja/healthz πππ |
HANDLER branch is deployed to platta: https://helsinkilisa-ui-handler-pr2925.dev.hel.ninja πππ |
|
|
|
|
|
|
API branch is deployed to platta: https://kesaseteli-pr2925.api.dev.hel.ninja/healthz πππ |
|
API branch is deployed to platta: https://tet-pr2925.api.dev.hel.ninja/healthz πππ |
|
|
ADMIN branch is deployed to platta: https://tet-admin-ui-pr2925.dev.hel.ninja πππ |
APPLICANT branch is deployed to platta: https://helsinkilisa-ui-pr2925.dev.hel.ninja πππ |
API branch is deployed to platta: https://helsinkilisa-pr2925.api.dev.hel.ninja/healthz πππ |
TestCafe result is success for https://tet-admin-ui-pr2925.dev.hel.ninja ππππ |
EMPLOYER branch is deployed to platta: https://kesaseteli-employer-pr2925.dev.hel.ninja πππ |
TestCafe result is success for https://helsinkilisa-ui-pr2925.dev.hel.ninja ππππ |
HANDLER branch is deployed to platta: https://helsinkilisa-ui-handler-pr2925.dev.hel.ninja πππ |
TestCafe result is success for https://kesaseteli-employer-pr2925.dev.hel.ninja ππππ |
TestCafe result is success for https://helsinkilisa-ui-handler-pr2925.dev.hel.ninja ππππ |
HANDLER branch is deployed to platta: https://kesaseteli-handler-pr2925.dev.hel.ninja πππ |
TestCafe result is success for https://kesaseteli-handler-pr2925.dev.hel.ninja ππππ |
YOUTH branch is deployed to platta: https://tet-youth-ui-pr2925.dev.hel.ninja πππ |
TestCafe result is success for https://tet-youth-ui-pr2925.dev.hel.ninja ππππ |
YOUTH branch is deployed to platta: https://kesaseteli-youth-pr2925.dev.hel.ninja πππ |
TestCafe result is success for https://kesaseteli-youth-pr2925.dev.hel.ninja ππππ |
Additional notes ποΈ
In this PR, the DELETE REST endpoint is restricted for the use of the applicant only. It will be enabled for handlers later once the handler side progresses to that point.