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

5264 r37 get employee's contract address #59

Merged
merged 9 commits into from
Mar 15, 2022

Conversation

daveb-hni
Copy link
Collaborator

Added R37 Transaction

Copy link
Collaborator

@weskubo-cgi weskubo-cgi left a comment

Choose a reason for hiding this comment

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

Just some minor feedback.
Security is missing but that should be obvious once brought up to date with main.

*
* Also used by R37 as the results required for R37 are a subset of those returned for Contract Inquiry so it can return the same result. This
* does not break overall security as currently all roles with permissions for R37(Get Group Member's Contract Address) also have permission
* for R40(Contract Inquiry).
*
* @param contractInquireRequest
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not related to this commit but the name of the param (in javadoc and method) doesn't match it's class. Inquire vs INquiry.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated param name.

@@ -75,7 +75,11 @@

/**
* Get MSP Coverage info for a Personal Health Number (PHN) of a group Inquiry
* Maps to the legacy R40.
* Maps to the legacy: R40.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Javadoc formatting is now different between this method and others. Most don't use the colon.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed colon

@@ -17,7 +17,8 @@
CANCEL_GROUP_MEMBER("CancelGroupMember"),
CANCEL_DEPENDENT("CancelDependent"),
GET_CONTRACT_PERIODS("GetContractPeriods"),
CONTRACT_INQUIRY("ContractInquiry"),
CONTRACT_INQUIRY("ContractInquiry"), //R40
GET_GROUP_MEMBERS_CONTRACT_ADDRESS("GetGroupMembersContractAddress"), //R37
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should just use GET_CONTRACT_ADDRESS(GetContactAddress) which matches the new terminology: GetContractAddress. The list of transactions is available on the Audit Design page.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated enum name & prop

@@ -46,6 +46,7 @@
<div class="dropdown-content">
<router-link @click="resetAlert" :class="menuClass($route, 'GetContractPeriods')" :to="{ name: 'GetContractPeriods' }">Get Contract Periods</router-link>
<router-link @click="resetAlert" :class="menuClass($route, 'ContractInquiry')" :to="{ name: 'ContractInquiry' }">Contract Inquiry</router-link>
<router-link @click="resetAlert" :class="menuClass($route, 'GetGroupMembersContractAddress')" :to="{ name: 'GetGroupMembersContractAddress' }">Get Group Member's Contract Address</router-link>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be secured.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added security

path: 'getGroupMembersContractAddress',
name: 'GetGroupMembersContractAddress',
component: GetGroupMembersContractAddress,
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be secured.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added security

<tbody>
<tr>
<!-- Only the first row should be shown as that will be the searched on Group Member -->
<GroupMemberContractBeneficiary :beneficiary="result.contractInquiryBeneficiaries[0]" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be cleaner to store the single row as data rather than the whole result. Nevermind, I see that other parts of the result are used. How about a computed field then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved to computed field

@@ -7,6 +7,7 @@ import SubNavTab from '../../components/template/SubNavTab.vue'
<TheSubNav>
<SubNavTab routeName="GetContractPeriods" title="Get Contract Periods" />
<SubNavTab routeName="ContractInquiry" title="Contract Inquiry" />
<SubNavTab routeName="GetGroupMembersContractAddress" title="Get Group Member's Contract Address" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be secured.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added security

@daveb-hni daveb-hni merged commit 6f43c40 into main Mar 15, 2022
@daveb-hni daveb-hni deleted the 5264-R37-Get-Employee's-Contract-Address branch March 15, 2022 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants