Skip to content

Address PR comments from PiCT (14) #254

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

Merged
merged 1 commit into from
Apr 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 26 additions & 0 deletions src/middlewares/isToggle.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { NextFunction, Request, Response } from "express";
import Toggle from "../models/misc/Toggle";
import logger from '../config/loggingConfig';

export const isToggle = (toggleName: string) => async (
req: Request,
res: Response,
next: NextFunction
) => {
try {
const toggle = await Toggle.findOne({ name: toggleName });

if (!toggle || !toggle.enabled) {
return res.status(403).json({
message: "Feature is currently disabled",
});
}

return next();
} catch (error) {
logger.error(`Failed to fetch toggle ${toggleName}:`, error);
return res.status(500).json({
message: `Failed to fetch toggle ${toggleName}; please try again later`
});
}
};
2 changes: 2 additions & 0 deletions src/models/SellerItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ const sellerItemSchema = new Schema<ISellerItem>(
}
);

sellerItemSchema.index({ seller_id: 1 });

// Creating the Seller model from the schema
const SellerItem = mongoose.model<ISellerItem>("Seller-Item", sellerItemSchema);

Expand Down
11 changes: 9 additions & 2 deletions src/routes/seller.routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Router } from "express";
import * as sellerController from "../controllers/sellerController";
import { isSellerFound } from "../middlewares/isSellerFound";
import { verifyToken } from "../middlewares/verifyToken";
import { isToggle } from "../middlewares/isToggle";
import upload from "../utils/multer";

/**
Expand Down Expand Up @@ -289,7 +290,11 @@ sellerRoutes.delete(
* 500:
* description: Internal server error
*/
sellerRoutes.get("/item/:seller_id", sellerController.getSellerItems);
sellerRoutes.get(
"/item/:seller_id",
isToggle("onlineShoppingFeature"),
sellerController.getSellerItems
);

/**
* @swagger
Expand Down Expand Up @@ -319,7 +324,8 @@ sellerRoutes.get("/item/:seller_id", sellerController.getSellerItems);
* description: Internal server error
*/
sellerRoutes.put(
"/item/add",
"/item/add",
isToggle("onlineShoppingFeature"),
verifyToken,
isSellerFound,
upload.single("image"),
Expand Down Expand Up @@ -358,6 +364,7 @@ sellerRoutes.put(
*/
sellerRoutes.delete(
"/item/delete/:item_id",
isToggle("onlineShoppingFeature"),
verifyToken,
isSellerFound,
sellerController.deleteSellerItem
Expand Down
103 changes: 59 additions & 44 deletions src/services/seller.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,45 +13,60 @@ import { IUser, IUserSettings, ISeller, ISellerWithSettings, ISellerItem, ISanct
import logger from "../config/loggingConfig";

// Helper function to get settings for all sellers and merge them into seller objects
const resolveSellerSettings = async (sellers: ISeller[], trustLevelFilters?: number[]): Promise<ISellerWithSettings[]> => {
const sellersWithSettings = await Promise.all(
sellers.map(async (seller) => {
try {
const sellerObject = seller.toObject();

// Fetch the user settings for the seller
const userSettings = await UserSettings.findOne({ user_settings_id: seller.seller_id }).exec();

// Check if the seller's trust level is allowed
if (trustLevelFilters && trustLevelFilters.includes(userSettings?.trust_meter_rating ?? -1)) {
return null; // Exclude this seller
}

// Merge seller and settings into a single object
return {
...sellerObject,
trust_meter_rating: userSettings?.trust_meter_rating,
user_name: userSettings?.user_name,
findme: userSettings?.findme,
email: userSettings?.email ?? null,
phone_number: userSettings?.phone_number ?? null,
search_filters: userSettings?.search_filters ?? null,
} as ISellerWithSettings;
} catch (error) {
logger.error(`Failed to resolve settings for sellerID ${ seller.seller_id }:`, error);

// Return a fallback seller object with minimal information
return {
...seller.toObject(),
trust_meter_rating: TrustMeterScale.ZERO,
user_name: seller.name,
findme: null,
email: null,
phone_number: null
} as unknown as ISellerWithSettings;
}
})
const resolveSellerSettings = async (
sellers: ISeller[],
trustLevelFilters?: number[]
): Promise<ISellerWithSettings[]> => {

if (!sellers.length) return [];

const sellerIds = sellers.map(seller => seller.seller_id);

// Batch fetch all relevant user settings in a single query
const allUserSettings = await UserSettings.find({
user_settings_id: { $in: sellerIds }
}).exec();

// Create a map for quick user settings lookup
const settingsMap = new Map(
allUserSettings.map(setting => [setting.user_settings_id, setting])
);

const sellersWithSettings = sellers.map((seller) => {
const sellerObject = seller.toObject();
const userSettings = settingsMap.get(seller.seller_id);

// Check if the seller's trust level is allowed
const trustMeterRating = userSettings?.trust_meter_rating ?? -1;
if (trustLevelFilters && !trustLevelFilters.includes(trustMeterRating)) {
return null; // Exclude this seller
}

try {
return {
...sellerObject,
trust_meter_rating: trustMeterRating,
user_name: userSettings?.user_name,
findme: userSettings?.findme,
email: userSettings?.email ?? null,
phone_number: userSettings?.phone_number ?? null,
search_filters: userSettings?.search_filters ?? null,
} as ISellerWithSettings;
} catch (error) {
logger.error(`Failed to resolve settings for sellerID ${ seller.seller_id }:`, error);

// Return a fallback seller object with minimal information
return {
...sellerObject,
trust_meter_rating: TrustMeterScale.ZERO,
user_name: seller.name,
findme: null,
email: null,
phone_number: null,
} as unknown as ISellerWithSettings;
}
});

return sellersWithSettings.filter(Boolean) as ISellerWithSettings[];
};

Expand All @@ -70,11 +85,11 @@ export const getAllSellers = async (
const baseCriteria: Record<string, any> = {};
const sellerTypeFilters: SellerType[] = [];

if (!searchFilters.include_active_sellers) sellerTypeFilters.push(SellerType.Active);
if (!searchFilters.include_inactive_sellers) sellerTypeFilters.push(SellerType.Inactive);
if (!searchFilters.include_test_sellers) sellerTypeFilters.push(SellerType.Test);
// exclude filtered seller types
if (sellerTypeFilters.length) baseCriteria.seller_type = { $nin: sellerTypeFilters };
if (searchFilters.include_active_sellers) sellerTypeFilters.push(SellerType.Active);
if (searchFilters.include_inactive_sellers) sellerTypeFilters.push(SellerType.Inactive);
if (searchFilters.include_test_sellers) sellerTypeFilters.push(SellerType.Test);
// include filtered seller types
if (sellerTypeFilters.length) baseCriteria.seller_type = { $in: sellerTypeFilters };

// Trust Level Filters
const trustLevels = [
Expand All @@ -84,7 +99,7 @@ export const getAllSellers = async (
{ key: "include_trust_level_0", value: TrustMeterScale.ZERO },
];
const trustLevelFilters = trustLevels
.filter(({ key }) => !searchFilters[key]) // exclude unchecked trust levels
.filter(({ key }) => searchFilters[key]) // Only include checked trust levels
.map(({ value }) => value);

// Search Query Filter
Expand Down
2 changes: 2 additions & 0 deletions test/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ beforeAll(async () => {

// Load the mock data into Map of PI DB collections
await User.insertMany(mockData.users);
await UserSettings.createIndexes();
await UserSettings.insertMany(mockData.userSettings);
// Ensure indexes are created for the schema models before running tests
await Seller.createIndexes();
await Seller.insertMany(mockData.sellers);
await SellerItem.createIndexes();
await SellerItem.insertMany(mockData.sellerItems);
await ReviewFeedback.insertMany(mockData.reviews);
await SanctionedRegion.insertMany(mockData.sanctionedRegion);
Expand Down
67 changes: 67 additions & 0 deletions test/middlewares/isToggle.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { NextFunction, Request, Response } from "express";
import { isToggle } from "../../src/middlewares/isToggle";
import Toggle from "../../src/models/misc/Toggle";

describe("isToggle function", () => {
let req: Partial<Request>;
let res: Partial<Response>;
let next: NextFunction;

beforeEach(() => {
req = {};
res = {
status: jest.fn().mockReturnThis(),
json: jest.fn(),
};
next = jest.fn();
});

it("should pass middleware if expected toggle is enabled", async () => {
const middleware = isToggle("testToggle_1");
await middleware(req as Request, res as Response, next);

expect(next).toHaveBeenCalled();
expect(res.status).not.toHaveBeenCalled();
});

it("should return 403 if expected toggle is disabled", async () => {
const middleware = isToggle("testToggle");
await middleware(req as Request, res as Response, next);

expect(res.status).toHaveBeenCalledWith(403);
expect(res.json).toHaveBeenCalledWith({
message: "Feature is currently disabled",
});
expect(next).not.toHaveBeenCalled();
});

it("should return 403 if expected toggle is not found", async () => {
const middleware = isToggle("testToggle_nonExisting");
await middleware(req as Request, res as Response, next);

expect(res.status).toHaveBeenCalledWith(403);
expect(res.json).toHaveBeenCalledWith({
message: "Feature is currently disabled",
});
expect(next).not.toHaveBeenCalled();
});

it("should return 500 if an exception occurs", async () => {
const toggleName = "testToggle_nonExisting";
const mockError = new Error(`Failed to fetch toggle ${ toggleName }; please try again later`);

const findOneSpy = jest.spyOn(Toggle, 'findOne').mockRejectedValue(mockError);

const middleware = isToggle(toggleName);
await middleware(req as Request, res as Response, next);

expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith({
message: `Failed to fetch toggle ${ toggleName }; please try again later`,
});
expect(next).not.toHaveBeenCalled();

// Restore original method to avoid affecting other tests
findOneSpy.mockRestore();
});
});
56 changes: 56 additions & 0 deletions test/mockData.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,62 @@
"phone_number": "555-555-5555",
"findme": "deviceGPS",
"trust_meter_rating": 100
},
{
"user_settings_id": "0f0f0f-0f0f-0f0f",
"user_name": "Test Six",
"email": "[email protected]",
"phone_number": "666-666-6666",
"findme": "deviceGPS",
"trust_meter_rating": 50
},
{
"user_settings_id": "0g0g0g-0g0g-0g0g",
"user_name": "Test Seven",
"email": "[email protected]",
"phone_number": "777-777-7777",
"findme": "deviceGPS",
"trust_meter_rating": 50
},
{
"user_settings_id": "0h0h0h-0h0h-0h0h",
"user_name": "Test Eight",
"email": "[email protected]",
"phone_number": "888-888-8888",
"findme": "deviceGPS",
"trust_meter_rating": 50
},
{
"user_settings_id": "0i0i0i-0i0i-0i0i",
"user_name": "Test Nine",
"email": "[email protected]",
"phone_number": "999-999-9999",
"findme": "deviceGPS",
"trust_meter_rating": 50
},
{
"user_settings_id": "0j0j0j-0j0j-0j0j",
"user_name": "Test Ten",
"email": "[email protected]",
"phone_number": "101-101-1010",
"findme": "deviceGPS",
"trust_meter_rating": 50
},
{
"user_settings_id": "0k0k0k-0k0k-0k0k",
"user_name": "Test Eleven",
"email": "[email protected]",
"phone_number": "111-111-1111",
"findme": "deviceGPS",
"trust_meter_rating": 50
},
{
"user_settings_id": "0l0l0l-0l0l-0l0l",
"user_name": "Test Twelve",
"email": "[email protected]",
"phone_number": "121-121-1212",
"findme": "deviceGPS",
"trust_meter_rating": 50
}
],
"sellers": [
Expand Down
2 changes: 1 addition & 1 deletion test/services/seller.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('getAllSellers function', () => {
const sellersData = await getAllSellers(undefined, undefined, userData.pi_uid);

// filter out inactive sellers and sellers with trust level <= 50.
expect(sellersData).toHaveLength(9);
expect(sellersData).toHaveLength(2);
});

it('should fetch all applicable sellers when search query is provided and bounding box params are empty', async () => {
Expand Down