OBPIH-5802 select approver in create request approvals end to end workflow #4286

drodzewicz · 2023-09-18T07:11:44Z

No description provided.

src/js/components/stock-movement-wizard/request/CreateStockMovement.jsx

kchelstowski · 2023-09-18T07:31:39Z

src/js/components/stock-movement-wizard/request/CreateStockMovement.jsx

@@ -198,6 +231,14 @@ class CreateStockMovement extends Component {
    if (this.state.values.origin && this.state.values.destination) {
      this.fetchStockLists(this.state.values.origin, this.state.values.destination);
    }
+    if (this.state.values.origin) {
+      if (this.state.values.origin?.supportedActivities?.includes(ActivityCode.APPROVE_REQUEST)) {


what about destination's supportedActivities? Has it been clarified what we should look at exactly?

src/js/components/stock-movement-wizard/request/CreateStockMovement.jsx

kchelstowski · 2023-09-18T07:33:15Z

src/js/components/stock-movement-wizard/request/CreateStockMovement.jsx

+
+        return response.data.data;
+      })
+      .catch(() => this.props.hideSpinner());


let's maybe add .finally here that would hide the spinner instead of hiding in both in .catch and in .then?

good idea, I was just copy/pasting the previous fetch methods in the component and followed the "convention"

kchelstowski · 2023-09-18T07:36:04Z

grails-app/controllers/org/pih/warehouse/api/SelectOptionsApiController.groovy

+            params.location = session.warehouse?.id
+        }
+        List<User> users = userService.findUsers(params).collect {
+            [id: it.id, label: it.name, value: it.id ]


maybe not for now, but maybe we should think about creating a dto called SelectOptionDto for those select options?
In all places we return the same object which is object with id, label, value.
It'd be easier to manage those select options if we ever wanted e.g. to add a new field to be returned etc.
But as I said - it's probably not for now, but it was a good moment to raise it.

Did the structure of the response change? If yes then it should be a separate endpoint.

awalkowiak · 2023-09-18T08:16:59Z

grails-app/conf/UrlMappings.groovy

@@ -82,7 +82,7 @@ class UrlMappings {
            action = [GET: "paymentTermOptions"]
        }

-        "/api/users" {
+        "/api/usersOptions" {


Why this change? You should not change an existing endpoint like that.

awalkowiak · 2023-09-18T08:17:05Z

grails-app/conf/UrlMappings.groovy

@@ -82,7 +82,7 @@ class UrlMappings {
            action = [GET: "paymentTermOptions"]
        }

-        "/api/users" {
+        "/api/usersOptions" {


Why this change? You should not change an existing endpoint like that.

In my opinion, this should have been done like that in the first place.
This is an endpoint from selectOptionsApiController and looking at previous "...Options" endpoints, this is the approach we went with for these kinds of endpoints.

Also, this endpoint is only used in debounce fetch for user-select which data was transformed on the frontend.

awalkowiak · 2023-09-18T08:37:14Z

grails-app/controllers/org/pih/warehouse/api/SelectOptionsApiController.groovy

+            params.location = session.warehouse?.id
+        }
+        List<User> users = userService.findUsers(params).collect {
+            [id: it.id, label: it.name, value: it.id ]


Did the structure of the response change? If yes then it should be a separate endpoint.

awalkowiak · 2023-09-18T08:37:22Z

grails-app/controllers/org/pih/warehouse/api/SelectOptionsApiController.groovy

+            params.location = session.warehouse?.id
+        }
+        List<User> users = userService.findUsers(params).collect {
+            [id: it.id, label: it.name, value: it.id ]


Did the structure of the response change? If yes then it should be a separate endpoint.

drodzewicz · 2023-09-18T13:24:04Z

As I mentioned before I still believe the previously used /api/users endpoint was supposed to return an option structured like array [label, id, value] like all of the other endpoints in SelectOptionsApiController and have a different endpoint name /api/userOptions just like others.
But to not confuse the API consumer which might have been using this endpoint, I created a UserApiController with list which returns exactly the same data as it returned in the SelectOptionsApiController.
So from the point of view of the Api consumer, nothing has changed.
@awalkowiak

Add ability to search users by RoleType

… of stockMovement response

- add const for roleTypes - conditionaly render approver multiselect if fulfiling location supports APPROVE_REQUEST - if only one approver is available then preselct this value by default

- fix userOptions endpoint to match other option endpoint names - move roleType parameter to other findUsers service method = transform userOptions on backend to label, value, id

awalkowiak · 2023-09-18T13:55:14Z

@drodzewicz but you still changed the url of this endpoint

…kflow (#4286) * OBPIH-5802 Add UserApiController Add ability to search users by RoleType * OBPIH-5802 Add supportedActivities property on origin and destination of stockMovement response * OBPIH-5802 Approver field on Create Request form - add const for roleTypes - conditionaly render approver multiselect if fulfiling location supports APPROVE_REQUEST - if only one approver is available then preselct this value by default * OBPIH-5802 Set initial values for approvers on existing request * OBPIH-5802 Remove UserAPIController and use userOptions endpoint instead - fix userOptions endpoint to match other option endpoint names - move roleType parameter to other findUsers service method = transform userOptions on backend to label, value, id * OBPIH-5802 Add subtext to approver select component * OBPIH-5802 Rollback usersOption list endpoint changes * OBPIH-5802 Add approvers list on stockMovement

drodzewicz requested review from awalkowiak, alannadolny and kchelstowski September 18, 2023 07:11

drodzewicz self-assigned this Sep 18, 2023

alannadolny reviewed Sep 18, 2023

View reviewed changes

src/js/components/stock-movement-wizard/request/CreateStockMovement.jsx Show resolved Hide resolved

drodzewicz force-pushed the OBPIH-5802-select-approver-in-create-request-approvals-end-to-end-workflow branch from a8a2638 to ac8c57b Compare September 18, 2023 07:30

kchelstowski reviewed Sep 18, 2023

View reviewed changes

awalkowiak requested changes Sep 18, 2023

View reviewed changes

drodzewicz requested a review from awalkowiak September 18, 2023 13:24

drodzewicz added 6 commits September 18, 2023 15:51

OBPIH-5802 Add UserApiController

d24916c

Add ability to search users by RoleType

OBPIH-5802 Add supportedActivities property on origin and destination…

1a6d9ba

… of stockMovement response

OBPIH-5802 Approver field on Create Request form

55180b8

- add const for roleTypes - conditionaly render approver multiselect if fulfiling location supports APPROVE_REQUEST - if only one approver is available then preselct this value by default

OBPIH-5802 Set initial values for approvers on existing request

2858e64

OBPIH-5802 Remove UserAPIController and use userOptions endpoint instead

6f3c8c3

- fix userOptions endpoint to match other option endpoint names - move roleType parameter to other findUsers service method = transform userOptions on backend to label, value, id

OBPIH-5802 Add subtext to approver select component

7e83a7e

drodzewicz added 2 commits September 18, 2023 17:41

OBPIH-5802 Rollback usersOption list endpoint changes

469dee3

OBPIH-5802 Add approvers list on stockMovement

e909ea6

drodzewicz force-pushed the OBPIH-5802-select-approver-in-create-request-approvals-end-to-end-workflow branch from 8183f3f to e909ea6 Compare September 18, 2023 16:16

awalkowiak approved these changes Sep 19, 2023

View reviewed changes

awalkowiak merged commit 55b6903 into develop Sep 19, 2023
3 checks passed

awalkowiak deleted the OBPIH-5802-select-approver-in-create-request-approvals-end-to-end-workflow branch September 19, 2023 08:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OBPIH-5802 select approver in create request approvals end to end workflow #4286

OBPIH-5802 select approver in create request approvals end to end workflow #4286

drodzewicz commented Sep 18, 2023

kchelstowski Sep 18, 2023

kchelstowski Sep 18, 2023

drodzewicz Sep 18, 2023

kchelstowski Sep 18, 2023

awalkowiak Sep 18, 2023

awalkowiak Sep 18, 2023

awalkowiak Sep 18, 2023

awalkowiak Sep 18, 2023

drodzewicz Sep 18, 2023

awalkowiak Sep 18, 2023

awalkowiak Sep 18, 2023

drodzewicz commented Sep 18, 2023

awalkowiak commented Sep 18, 2023

OBPIH-5802 select approver in create request approvals end to end workflow #4286

OBPIH-5802 select approver in create request approvals end to end workflow #4286

Conversation

drodzewicz commented Sep 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drodzewicz commented Sep 18, 2023

awalkowiak commented Sep 18, 2023