Skip to content
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

[OCRVS-3148] Hide registrar signature from field agents #3200

Merged
merged 7 commits into from May 25, 2022
Merged

Conversation

rikukissa
Copy link
Collaborator

@rikukissa rikukissa commented May 24, 2022

I tried writing a test for this, but there's just so many FHIR requests being made in type resolvers, that I gave up

@codecov
Copy link

codecov bot commented May 24, 2022

Codecov Report

Merging #3200 (589a1b5) into develop (e3d8c8c) will increase coverage by 0.02%.
The diff coverage is 88.31%.

@@             Coverage Diff             @@
##           develop    #3200      +/-   ##
===========================================
+ Coverage    79.87%   79.90%   +0.02%     
===========================================
  Files          605      606       +1     
  Lines        27785    27848      +63     
  Branches      8841     8858      +17     
===========================================
+ Hits         22194    22252      +58     
- Misses        5536     5541       +5     
  Partials        55       55              
Impacted Files Coverage Δ
packages/client/src/user/queries.ts 42.85% <ø> (ø)
...es/client/src/views/Performance/FieldAgentList.tsx 70.27% <ø> (ø)
...src/views/SysAdmin/Performance/PerformanceHome.tsx 64.96% <ø> (ø)
.../src/views/SysAdmin/Performance/WorkflowStatus.tsx 83.71% <ø> (ø)
packages/auth/src/server.ts 82.14% <66.66%> (-0.88%) ⬇️
...ckages/gateway/src/features/user/root-resolvers.ts 95.00% <66.66%> (-1.03%) ⬇️
...ckages/gateway/src/features/user/type-resolvers.ts 91.66% <66.66%> (-2.09%) ⬇️
packages/gateway/src/server.ts 71.42% <66.66%> (-0.37%) ⬇️
.../gateway/src/features/correction/root-resolvers.ts 92.59% <75.00%> (-7.41%) ⬇️
packages/gateway/src/utils/validators.ts 97.14% <97.14%> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1144b04...589a1b5. Read the comment docs.

@cypress
Copy link

cypress bot commented May 24, 2022



Test summary

29 0 0 0Flakiness 0


Run details

Project OpenCRVS Core
Status Passed
Commit b892482 ℹ️
Started May 25, 2022 12:31 PM
Ended May 25, 2022 12:42 PM
Duration 11:18 💡
OS Linux Ubuntu - 20.04
Browser Chrome 101

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

Copy link
Collaborator

@Zangetsu101 Zangetsu101 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that we don't actually use the signature returned from here.

export function getUserDetails(user: GQLUser): IUserDetails {
const {
catchmentArea,
primaryOffice,
name,
mobile,
role,
type,
status,
userMgntUserID,
practitionerId,
localRegistrar,
avatar
} = user
const userDetails: IUserDetails = {}
if (localRegistrar) {
userDetails.localRegistrar = localRegistrar
}
if (userMgntUserID) {
userDetails.userMgntUserID = userMgntUserID
}
if (practitionerId) {
userDetails.practitionerId = practitionerId
}
if (name) {
userDetails.name = name
}
if (mobile) {
userDetails.mobile = mobile
}
if (role) {
userDetails.role = role
}
if (type) {
userDetails.type = type
}
if (status) {
userDetails.status = status
}
if (primaryOffice) {
userDetails.primaryOffice = {
id: primaryOffice.id,
name: primaryOffice.name,
alias: primaryOffice.alias,
status: primaryOffice.status
}
}
if (catchmentArea) {
const areaWithLocations: GQLLocation[] = catchmentArea as GQLLocation[]
const potentialCatchmentAreas = areaWithLocations.map(
(cArea: GQLLocation) => {
if (cArea.identifier) {
const identifiers: GQLIdentifier[] =
cArea.identifier as GQLIdentifier[]
return {
id: cArea.id,
name: cArea.name,
alias: cArea.alias,
status: cArea.status,
identifier: identifiers.map((identifier: GQLIdentifier) => {
return {
system: identifier.system,
value: identifier.value
}
})
}
}
return {}
}
) as IGQLLocation[]
if (potentialCatchmentAreas !== undefined) {
userDetails.catchmentArea = potentialCatchmentAreas
}
}
if (avatar) {
userDetails.avatar = avatar
}
return userDetails
}

Maybe we can remove it from the schema?
IUserDetails doesn't have any signature field.

@rikukissa
Copy link
Collaborator Author

Isn't the signature part of the localRegistrar key? I might not understand what you mean

export interface IUserDetails {
  userMgntUserID?: string
  practitionerId?: string
  mobile?: string
  role?: string
  type?: string
  status?: string
  name?: Array<GQLHumanName | null>
  catchmentArea?: IGQLLocation[]
  primaryOffice?: IGQLLocation
  localRegistrar?: {
    name: Array<GQLHumanName | null>
    role?: string
    signature?: GQLSignature
  }
  avatar?: IAvatar
}

@Zangetsu101
Copy link
Collaborator

Isn't the signature part of the localRegistrar key? I might not understand what you mean

export interface IUserDetails {
  userMgntUserID?: string
  practitionerId?: string
  mobile?: string
  role?: string
  type?: string
  status?: string
  name?: Array<GQLHumanName | null>
  catchmentArea?: IGQLLocation[]
  primaryOffice?: IGQLLocation
  localRegistrar?: {
    name: Array<GQLHumanName | null>
    role?: string
    signature?: GQLSignature
  }
  avatar?: IAvatar
}

I was talking about this one

@rikukissa
Copy link
Collaborator Author

I removed the signature field now. We might need to add it back if we implement a feature for sysadmins to update signatures or something like that. For now it's good though 👍

jest wasnt waiting for a test in registration/root-resolvers and it was silently failing
@rikukissa rikukissa merged commit 9aef304 into develop May 25, 2022
@rikukissa rikukissa deleted the ocrvs-3148 branch May 25, 2022 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants