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.

Sign up for GitHub

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

Jump to bottom

ATS Client Push #184

Merged
merged 2 commits into from
Nov 4, 2024
Merged

ATS Client Push #184

merged 2 commits into from
Nov 4, 2024

Conversation

sanjaytkbabu
Copy link
Contributor

@sanjaytkbabu sanjaytkbabu commented Oct 18, 2024 •
edited
Loading

Description

PADS-231 - ATS Client Push API
PADS-317 - ATS Client Push UI

ATS client push UI:
New ATSUserCreateModal
New frontend type for ATSClientResource
SubmissionForm, ATSUserLinkModal, ATSUserDetailModal, atsService updated
ATS client push API:
New POST API for creating clients in ATS
New backend type ATSClientResource, ATSUserSearchParameters
atsController, route, services, validator updated
Backend unit test added

Types of changes

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 18, 2024 •
edited
Loading

Coverage Report (Frontend)

Totals Coverage
Statements: 25.76% ( 1585 / 6153 )
Methods: 20.9% ( 223 / 1067 )
Lines: 30.22% ( 920 / 3044 )
Branches: 21.65% ( 442 / 2042 )

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 18, 2024 •
edited
Loading

Coverage Report (Application)

Totals Coverage
Statements: 36.65% ( 964 / 2630 )
Methods: 26.34% ( 118 / 448 )
Lines: 47.74% ( 645 / 1351 )
Branches: 24.19% ( 201 / 831 )

@sanjaytkbabu sanjaytkbabu marked this pull request as ready for review October 18, 2024 21:23
@sanjaytkbabu sanjaytkbabu force-pushed the feature/ats-client-push branch 9 times, most recently from 73ad279 to 5d51b35 Compare October 22, 2024 21:12
kyle1morel
kyle1morel reviewed Oct 22, 2024
View reviewed changes
app/src/controllers/ats.ts Outdated Show resolved Hide resolved
app/src/routes/v1/ats.ts Outdated Show resolved Hide resolved
app/src/services/ats.ts Outdated Show resolved Hide resolved
app/src/validators/ats.ts Outdated Show resolved Hide resolved
app/src/validators/ats.ts Outdated Show resolved Hide resolved
frontend/src/components/user/ATSUserCreateModal.vue Outdated Show resolved Hide resolved
frontend/src/components/user/ATSUserCreateModal.vue Outdated Show resolved Hide resolved
frontend/src/components/user/ATSUserLinkModal.vue Outdated Show resolved Hide resolved
frontend/src/components/user/ATSUserLinkModal.vue Outdated Show resolved Hide resolved
frontend/src/components/user/ATSUserLinkModal.vue Outdated Show resolved Hide resolved
@sanjaytkbabu sanjaytkbabu force-pushed the feature/ats-client-push branch 3 times, most recently from 19fa94b to 822f76f Compare October 23, 2024 22:35
kyle1morel
kyle1morel reviewed Oct 23, 2024
View reviewed changes
app/src/types/ATSClientResource.ts Outdated Show resolved Hide resolved
app/src/types/ATSClientResource.ts Outdated Show resolved Hide resolved
app/tests/unit/controllers/ats.spec.ts Outdated Show resolved Hide resolved
frontend/src/components/user/ATSUserCreateModal.vue Outdated Show resolved Hide resolved
frontend/src/components/user/ATSUserLinkModal.vue Show resolved Hide resolved
frontend/src/services/atsService.ts Outdated Show resolved Hide resolved
@sanjaytkbabu sanjaytkbabu force-pushed the feature/ats-client-push branch 4 times, most recently from 2f2f0a8 to 15fc73f Compare October 24, 2024 22:26
@sanjaytkbabu sanjaytkbabu force-pushed the feature/ats-client-push branch 3 times, most recently from 258a1a5 to e4ace83 Compare October 29, 2024 20:55
kyle1morel
kyle1morel reviewed Oct 30, 2024
View reviewed changes
frontend/src/components/user/ATSUserCreateModal.vue Outdated
toast.error('Error pushing client to ATS');
}
} catch (error) {
if (!axios.isCancel(error)) toast.error('Error pushing client to ATS ' + error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

No cancel token is being used. CancelToken has also deprecated since v0.22.0

frontend/src/components/user/ATSUserDetailsModal.vue Outdated
address: address
};
client.formattedAddress = address;
client.phone = client.address.primaryPhone ?? client.address.secondaryPhone;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is client.phone part of the original response? if not we should be referencing it through the proper keys instead of duplicating it.

frontend/src/components/user/ATSUserLinkModal.vue Outdated
address: address
};
client.formattedAddress = address;
client.phone = client.address.primaryPhone ?? client.address.secondaryPhone;
Copy link
Collaborator

Choose a reason for hiding this comment

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

See above

frontend/src/components/user/ATSUserLinkModal.vue Outdated
address: address
};
client.formattedAddress = address;
client.phone = client.address.primaryPhone ?? client.address.secondaryPhone;
});
} catch (error) {
if (!axios.isCancel(error)) toast.error('Error searching for users ' + error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

See above about cancel token

@sanjaytkbabu
Copy link
Contributor Author

Sample Client Object (sanitized) for reference:
{
"@type": "ClientResource",
"links": [
{
"@type": "RelLink",
"rel": "self",
"href": "https://i1api.nrs.gov.bc.ca/ats-api/clients",
"method": "GET"
}
],
"clientId": 33767,
"address": {
"@type": "AddressResource",
"links": [],
"addressId": 50123,
"addressLine1": "295 LH Road",
"addressLine2": null,
"city": "Victoria",
"provinceCode": "British Columbia",
"countryCode": "Canada",
"postalCode": "V8H 1J9",
"primaryPhone": null,
"secondaryPhone": null,
"fax": null,
"email": null,
"createdBy": null,
"createdDateTime": null,
"updatedBy": null,
"updatedDateTime": null
},
"businessOrgCode": null,
"firstName": "Jane",
"surName": "Doe",
"companyName": null,
"organizationNumber": null,
"confirmedIndicator": false,
"createdBy": "IDIR\JDOE",
"createdDateTime": 1219175659000,
"updatedBy": "IDIR\DILL",
"updatedDateTime": 1353650995000,
"regionName": "West Coast",
"optOutOfBCStatSurveyInd": "NO"
}

@sanjaytkbabu
ATS client push UI: New ATSUserCreateModal, SubmissionForm, ATSUserLi…
62c76f9 
…nkModal, atsService updated
@sanjaytkbabu
ATS client push API: New POST API, New backend type for ATSClient, at…
e715fe8 
…sController, route, services, validator updated

Backend unti test added
New ATSClientResource type, frontend components updated
@sanjaytkbabu sanjaytkbabu force-pushed the feature/ats-client-push branch from e4ace83 to e715fe8 Compare October 31, 2024 16:23
@wilwong89 wilwong89 merged commit 408ceb9 into release/interregnum Nov 4, 2024
16 of 17 checks passed
@wilwong89 wilwong89 deleted the feature/ats-client-push branch November 4, 2024 17:21
@kyle1morel kyle1morel mentioned this pull request Nov 27, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kyle1morel kyle1morel kyle1morel approved these changes

@wilwong89 wilwong89 wilwong89 approved these changes

@qhanson55 qhanson55 Awaiting requested review from qhanson55

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sanjaytkbabu @wilwong89 @kyle1morel