Use Controller in Meter, User, UserGroup, and ValueListGroup components. - #932
Use Controller in Meter, User, UserGroup, and ValueListGroup components.#932nbeatty-gpa wants to merge 14 commits into
Conversation
363a0c3 to
5e45b5d
Compare
86af343 to
de95926
Compare
| const [refreshTrigger, setRefreshTrigger] = React.useState<boolean>(false); | ||
|
|
||
| const [status, setStatus] = React.useState<Application.Types.Status>('uninitiated'); | ||
| const [groupStatus, setGroupStatus] = React.useState<Application.Types.Status>('uninitiated'); |
There was a problem hiding this comment.
This was added but not used, was that intentional?
|
|
||
| const handle = $.ajax({ | ||
| type: "POST", | ||
| url: `${homePath}api/SystemCenter/FullSecurityGroup/Users/${props.Group.ID}`, |
There was a problem hiding this comment.
This maps to the Users/{groupID} route in the SecurityGroup.cs model and it only accepts a get request. It also returns an array however after parsing its casted to a string. Which means that existing users never populate groupUsers. After clicking the 'Add Users' confirmation the existing memberships would get removed. The request needs to be changed to the proper type, the typings for groupUsers should change and we probably should disable confirmation until the existing memberships load succesfully.
There was a problem hiding this comment.
I'm having some trouble understanding what you mean here. The problem with the request type I understand, but I'm casting the result to unknown, then to string, so that the value can be parsed with JSON.parse, and I've tested that it works this way but does not work without JSON.parse. Disabling confirmation of the selection until the existing memberships load successfully makes sense, but I'm not sure what typing change for groupUsers you have in mind here
There was a problem hiding this comment.
Oops, i looked at that casting part wrong. I thought it was being casted outside the parse.
de95926 to
f6cc0ea
Compare
| async: true | ||
| }).done((d) => { | ||
| setGroupStatus('idle'); | ||
| setGroupUsers(JSON.parse(d.Data as unknown as string)); |
There was a problem hiding this comment.
The response is actually an array. So this is going to throw when trying to parse undefined.
| const [errors, setError] = React.useState<string[]>([]); | ||
| const [hover, setHover] = React.useState<('None' | 'Clear' | 'Save')>('None'); | ||
|
|
||
| const [refreshTrigger, setRefreshTrigger] = React.useState<boolean>(false); |
There was a problem hiding this comment.
The new refreshTrigger only reruns the warnings effect; it doesn’t fetch the saved group or update props.Group like the previous Redux flow did. After saving, warnings still remain, Clear Changes restores the old values, and switching tabs reloads the stale record. We should retrigger the parent’s fetch after a successful PATCH.
| const [showVirtualChannelModal, setShowVirtualChannelModal] = React.useState<boolean>(false); | ||
| const [spareList, setSpareList] = React.useState<string[]>([]); | ||
| const [listStatus, setListStatus] = React.useState<Application.Types.Status>('idle'); | ||
| const [refreshTrigger, setRefreshTrigger] = React.useState<boolean>(false); |
There was a problem hiding this comment.
The new refreshTrigger only refreshes the spare-channel list and reattaches the file-input handler; it doesn’t refresh the three lookup lists like the previous Redux flow did. After parsing, newly created phases, measurement types, and characteristics still won’t appear in the dropdowns. We should pass a refresh counterargument to all three useControllerFetch calls.
645964b to
d70e633
Compare
Removes the user account, security group, and remote user account slices, adds a
ControllerSelectPopupcomponent to replace the gemstone slice-basedSelectionPopup, and removes unused slices.