Skip to content

[CHORE]: reduce number of queries for location table#122

Merged
chelsea-EYDS merged 4 commits intomainfrom
locations-norm
Jun 7, 2023
Merged

[CHORE]: reduce number of queries for location table#122
chelsea-EYDS merged 4 commits intomainfrom
locations-norm

Conversation

@chelsea-EYDS
Copy link
Contributor

CHORE

Objective:

  • use "reduce" to create a nested location interface to use during reconciliation and reporting (instead of querying the location service in each of the deposit services by the location_id)

@chelsea-EYDS chelsea-EYDS requested a review from fw-noel June 3, 2023 21:42
@chelsea-EYDS chelsea-EYDS marked this pull request as ready for review June 3, 2023 21:42
Copy link
Collaborator

@fw-noel fw-noel left a comment

Choose a reason for hiding this comment

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

I like this PR a lot! Please address comments

});
return this.normalizeLocations(locations);
}
public normalizeLocations(locations: LocationEntity[]): NormalizedLocation[] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done (not showing here as outdated but can be seen in diff...)
Screenshot 2023-06-06 at 7 08 05 PM

description: '',
};
}
itm.merchant_id !== 999999999 &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know why because you explained it to me, but this should be commented on why. We may also want to define 999999999 as a constant. We always want to avoid magic numbers

}
itm.merchant_id !== 999999999 &&
acc[key].merchant_ids.push(itm.merchant_id);
if (itm.method === 'Bank') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

use LocationMethod enum since it exists?

@chelsea-EYDS chelsea-EYDS requested a review from fw-noel June 6, 2023 18:14
@chelsea-EYDS chelsea-EYDS merged commit 6d38799 into main Jun 7, 2023
@chelsea-EYDS chelsea-EYDS deleted the locations-norm branch June 7, 2023 18:16
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.

2 participants