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

feat: integrate FLSS with ERL email #5743

Closed
wants to merge 3 commits into from
Closed

feat: integrate FLSS with ERL email #5743

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7262420
feat: erl email integration with loanRecommendations endpoint
michelleinez Dec 19, 2024
c0f32ef
feat: added support for a couple more endpoint types
michelleinez Dec 19, 2024
5016ca2
fix: pr comments addressed
michelleinez Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 40 additions & 14 deletions server/live-loan-router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import express from 'express';
import { error } from './util/log.js';
import { getFromCache, setToCache } from './util/memJsUtils.js';
import drawLoanCard from './util/live-loan/live-loan-draw.js';
import fetchLoansByType from './util/live-loan/live-loan-fetch.js';
import fetchLoansByType, { QUERY_TYPE } from './util/live-loan/live-loan-fetch.js';
import { trace } from './util/mockTrace.js';

async function fetchRecommendedLoans(type, id, cache, flss = false) {
const loanCachedName = flss ? `recommendations-by-${type}-id-${id}-flss` : `recommendations-by-${type}-id-${id}`;
async function fetchRecommendedLoans(type, id, cache, queryType = QUERY_TYPE.DEFAULT) {
const queryTypeSuffix = queryType !== QUERY_TYPE.DEFAULT ? `-${queryType}` : '';
const loanCachedName = `recommendations-by-${type}-id-${id}${queryTypeSuffix}`;
// If we have loan data in memjscache return that quickly
try {
const cachedLoanData = await getFromCache(loanCachedName, cache);
Expand All @@ -18,7 +19,7 @@ async function fetchRecommendedLoans(type, id, cache, flss = false) {
}

// Otherwise we need to hit the graphql endpoint.
const loanData = await trace('fetchLoansByType', { resource: type }, async () => fetchLoansByType(type, id, flss)); // eslint-disable-line max-len
const loanData = await trace('fetchLoansByType', { resource: type }, async () => fetchLoansByType(type, id, queryType === QUERY_TYPE.FLSS, queryType === QUERY_TYPE.RECOMMENDATIONS)); // eslint-disable-line max-len
michelleinez marked this conversation as resolved.
Show resolved Hide resolved

// Set the loan data in memcache, return the loan data
if (loanData && loanData.length) {
Expand All @@ -34,12 +35,15 @@ async function fetchRecommendedLoans(type, id, cache, flss = false) {
throw new Error('No loans returned');
}

async function getLoanForRequest(type, cache, req, flss = false) {
async function getLoanForRequest(type, cache, req, queryType = QUERY_TYPE.DEFAULT) {
// Use default values for id and offset if they are not numeric
const id = req.params?.id || 0;
const offset = req.params?.offset || 1;

const loanData = await trace('fetchRecommendedLoans', async () => fetchRecommendedLoans(type, id, cache, flss));
const loanData = await trace(
'fetchRecommendedLoans',
async () => fetchRecommendedLoans(type, id, cache, queryType)
);

// if there are fewer loan results than the offset, return the last result
if (offset > loanData.length) {
Expand All @@ -48,9 +52,9 @@ async function getLoanForRequest(type, cache, req, flss = false) {
return loanData[offset - 1];
}

async function redirectToUrl(type, cache, req, res, flss = false) {
async function redirectToUrl(type, cache, req, res, queryType = QUERY_TYPE.DEFAULT) {
try {
const loan = await trace('getLoanForRequest', async () => getLoanForRequest(type, cache, req, flss));
const loan = await trace('getLoanForRequest', async () => getLoanForRequest(type, cache, req, queryType));
// Standard destination is the borrower profile page
let redirect = `/lend/${loan.id}`;
// If the original request had query params on it, forward those along
Expand All @@ -72,11 +76,12 @@ async function redirectToUrl(type, cache, req, res, flss = false) {
}
}

async function serveImg(type, style, cache, req, res, flss = false) {
async function serveImg(type, style, cache, req, res, queryType = QUERY_TYPE.DEFAULT) {
try {
const loan = await trace('getLoanForRequest', async () => getLoanForRequest(type, cache, req, flss));
const loan = await trace('getLoanForRequest', async () => getLoanForRequest(type, cache, req, queryType));
let loanImg;
const imgCachedName = flss ? `loan-card-img-${style}-${loan.id}-flss` : `loan-card-img-${style}-${loan.id}`;
const queryTypeSuffix = queryType !== QUERY_TYPE.DEFAULT ? `-${queryType}` : '';
const imgCachedName = `loan-card-img-${style}-${loan.id}${queryTypeSuffix}`;
const cachedLoanImg = await getFromCache(imgCachedName, cache);
if (cachedLoanImg) {
loanImg = cachedLoanImg;
Expand Down Expand Up @@ -139,21 +144,42 @@ export default function liveLoanRouter(cache) {
// User URL Router FLSS
router.use('/flss/u/:id(\\d{0,})/url/:offset(\\d{0,})', async (req, res) => {
await trace('live-loan.flss.user.redirectToUrl', { resource: req.path }, async () => {
await redirectToUrl('user', cache, req, res, true);
await redirectToUrl('user', cache, req, res, QUERY_TYPE.FLSS);
});
});

// User IMG Router FLSS (Legacy)
router.use('/flss/u/:id(\\d{0,})/img/:offset(\\d{0,})', async (req, res) => {
await trace('live-loan.flss.user.serveImg', { resource: req.path }, async () => {
await serveImg('user', 'legacy', cache, req, res, true);
await serveImg('user', 'legacy', cache, req, res, QUERY_TYPE.FLSS);
});
});

// User IMG Router FLSS (Kiva Classic)
router.use('/flss/u/:id(\\d{0,})/img2/:offset(\\d{0,})', async (req, res) => {
await trace('live-loan.flss.user.serveImg', { resource: req.path }, async () => {
await serveImg('user', 'classic', cache, req, res, true);
await serveImg('user', 'classic', cache, req, res, QUERY_TYPE.FLSS);
});
});

// User URL Router FLSS
michelleinez marked this conversation as resolved.
Show resolved Hide resolved
router.use('/recommendations/u/:id(\\d{0,})/url/:offset(\\d{0,})', async (req, res) => {
await trace('live-loan.flss.user.redirectToUrl', { resource: req.path }, async () => {
await redirectToUrl('user', cache, req, res, QUERY_TYPE.RECOMMENDATIONS);
});
});

// User IMG Router Recommendations (Legacy)
router.use('/recommendations/u/:id(\\d{0,})/img/:offset(\\d{0,})', async (req, res) => {
await trace('live-loan.recommendations.user.serveImg', { resource: req.path }, async () => {
await serveImg('user', 'legacy', cache, req, res, QUERY_TYPE.RECOMMENDATIONS);
});
});

// User IMG Router Recommendations (Kiva Classic)
router.use('/recommendations/u/:id(\\d{0,})/img2/:offset(\\d{0,})', async (req, res) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I tried this locally and encountered a server error. Is the new endpoint live and ready?

URL: https://kiva-ui.local/live-loan/recommendations/u/2230848/img2/1
Error on console locally: Error serving image, Error: No loans returned
Working alternative (old FLSS): https://kiva-ui.local/live-loan/flss/u/2230848/img2/1

Copy link
Collaborator

@mcstover mcstover Dec 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Was about to approve then saw this :)

I can make a basic query with that user id and it works ok in dev and prod...
image

await trace('live-loan.recommendations.user.serveImg', { resource: req.path }, async () => {
await serveImg('user', 'classic', cache, req, res, QUERY_TYPE.RECOMMENDATIONS);
});
});

Expand Down
33 changes: 29 additions & 4 deletions server/util/live-loan/live-loan-fetch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import fetchGraphQL from '../fetchGraphQL.js';
import { warn, error } from '../log.js';

export const QUERY_TYPE = {
DEFAULT: 'default',
FLSS: 'flss',
RECOMMENDATIONS: 'recommendations'
};

// Number of loans to fetch
const loanCount = 4;

Expand Down Expand Up @@ -60,8 +66,27 @@ async function fetchLoansFromGraphQL(request, resultPath) {
}

// Get per-user recommended loans from the ML service
async function fetchRecommendationsByLoginId(id, flss = false) {
if (flss) {
async function fetchRecommendationsByLoginId(id, queryType = QUERY_TYPE.DEFAULT) {
if (queryType === QUERY_TYPE.RECOMMENDATIONS) {
return fetchLoansFromGraphQL(
{
query: `query($userId: Int, $limit: Int) {
loanRecommendations(
userId: $userId,
limit: ${loanCount},
origin: "email:live-loans"
) {
${loanValues}
}
}`,
variables: {
userId: Number(id),
limit: loanCount
Copy link
Collaborator

Choose a reason for hiding this comment

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

@michelleinez @dyersituations There might be an issue here with loanCount...it's used below but not defined like this...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't it defined up above on line 11? It also looks like it was originally used in this same exact way down below in the FLSS fetch query...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree it appears to be! But you,ll want to confirm the actual query operation and combination of variables and that the failure isn't somewhere in the code setup...

}
},
'data.loanRecommendations.values'
);
} if (queryType === QUERY_TYPE.FLSS) {
return fetchLoansFromGraphQL(
{
query: `query($userId: Int) {
Expand Down Expand Up @@ -495,9 +520,9 @@ const shouldUseFLSS = async filterString => {
};

// Export a function that will fetch loans by live-loan type and id
export default async function fetchLoansByType(type, id, flss = false) {
export default async function fetchLoansByType(type, id, queryType = QUERY_TYPE.DEFAULT) {
if (type === 'user') {
return fetchRecommendationsByLoginId(id, flss);
return fetchRecommendationsByLoginId(id, queryType);
} if (type === 'loan') {
return fetchRecommendationsByLoanId(id);
} if (type === 'filter') {
Expand Down
18 changes: 17 additions & 1 deletion test/unit/specs/server/live-loan-fetch.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ describe('live-loan-fetch', () => {
fetch.default.mockClear();
fetch.default.mockResolvedValue({ json: () => { } });

await fetchLoansByType.default('user', '1234', true);
await fetchLoansByType.default('user', '1234', 'flss');

const { variables, query } = JSON.parse(fetch.default.mock.calls[0][1].body);
expect(fetch).toBeDefined();
Expand All @@ -173,4 +173,20 @@ describe('live-loan-fetch', () => {
expect(fetch.default.mock.results[0].value).toBeDefined();
});
});

describe('fetchRecommendationsByLoanRecs', () => {
it('should make recommendations call with correct parameters', async () => {
fetch.default.mockClear();
fetch.default.mockResolvedValue({ json: () => { } });

await fetchLoansByType.default('user', '1234', 'recommendations');

const { variables, query } = JSON.parse(fetch.default.mock.calls[0][1].body);
expect(fetch).toBeDefined();
expect(variables.userId).toEqual(1234);
expect(query).toBeDefined();
expect(query).toContain('loanRecommendations');
expect(fetch.default.mock.results[0].value).toBeDefined();
});
});
});
Loading