Online shop/membership #292
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| import { Request, Response } from "express"; | ||
| import * as membershipService from "../services/membership.service"; | ||
| import logger from "../config/loggingConfig"; | ||
| import { IUser } from "../types" | ||
|
|
||
| export const fetchMembershipList = async (req: Request, res: Response) => { | ||
| try { | ||
| const membershipList = await membershipService.buildMembershipList(); | ||
|
|
||
| if (!membershipList) { | ||
| logger.warn(`No membership list found.`); | ||
| return res.status(404).json({ message: "Membership list not found" }); | ||
| } | ||
|
|
||
| logger.info(`Fetched membership list`); | ||
| return res.status(200).json(membershipList); | ||
|
|
||
| } catch (error) { | ||
| logger.error(`Error getting membership list: `, error); | ||
| return res.status(500).json({ message: 'An error occurred while getting single membership list; please try again later' }); | ||
| } | ||
| }; | ||
|
|
||
| export const fetchUserMembership = async (req: Request, res: Response) => { | ||
| const authUser = req.currentUser as IUser; | ||
|
|
||
| try { | ||
| const currentMembership = await membershipService.getUserMembership(authUser); | ||
|
|
||
| if (!currentMembership) { | ||
| logger.warn(`Membership with ID ${authUser.pi_uid} not found.`); | ||
| return res.status(404).json({ message: "Membership not found" }); | ||
| } | ||
|
|
||
| logger.info(`Fetched membership with ID ${authUser.pi_uid}`); | ||
| return res.status(200).json(currentMembership); | ||
| } catch (error) { | ||
| logger.error(`Error getting membership ID ${authUser.pi_uid}:`, error); | ||
| return res.status(500).json({ message: 'An error occurred while getting single membership; please try again later' }); | ||
| } | ||
| }; | ||
|
|
||
|
|
||
| // Controller to fetch a single membership by its ID | ||
| export const getSingleMembership = async (req: Request, res: Response) => { | ||
| const { membership_id } = req.params; | ||
|
|
||
| try { | ||
| const membership = await membershipService.getSingleMembershipById(membership_id); | ||
|
|
||
| if (!membership) { | ||
| logger.warn(`Membership with ID ${membership_id} not found.`); | ||
| return res.status(404).json({ message: "Membership not found" }); | ||
| } | ||
|
|
||
| logger.info(`Fetched membership with ID ${membership_id}`); | ||
| return res.status(200).json(membership); | ||
| } catch (error) { | ||
| logger.error(`Error getting membership ID ${membership_id}:`, error); | ||
| return res.status(500).json({ message: 'An error occurred while getting single membership; please try again later' }); | ||
| } | ||
| }; | ||
|
|
||
| export const updateOrRenewMembership = async (req: Request, res: Response) => { | ||
| try { | ||
| const { membership_class } = req.body; | ||
| const authUser = req.currentUser | ||
|
|
||
| if (!authUser) return res.status(401).json({ error: "Unauthorized - user not authenticated "}); | ||
|
|
||
| const updated = await membershipService.updateOrRenewMembership(authUser.pi_uid, membership_class); | ||
|
|
||
| return res.status(200).json(updated); | ||
| } catch (error: any) { | ||
| logger.error("Failed to update or renew membership:", error); | ||
| return res.status(400).json({ error: error.message }); | ||
| } | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There was a problem hiding this comment. Nice work.
There was a problem hiding this comment. There are quite a number of anti-patterns in this helper file that will make it harder to maintain long-term i.e., tight coupling, duplication, and overall bloated code. Even just getting the unit tests rewritten to fix the CI/CD pipeline was a bit of a struggle because of the very procedural code structure. 😵 That said, I think it's better we take the time to clean it up now rather than let technical debt pile up. I've already started refactoring on a separate branch, so I'll plan to consolidate that work into this one and update the PR—but I'll do that after I finish reviewing the rest of the changes here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import { NextFunction, Request, Response } from "express"; | ||
| import Membership from "../models/Membership"; | ||
| import { IMembership } from "../types"; | ||
| import logger from '../config/loggingConfig'; | ||
|
|
||
| declare module 'express-serve-static-core' { | ||
| interface Request { | ||
| currentMembership: IMembership; | ||
| } | ||
| } | ||
|
|
||
| export const isMembershipFound = async ( | ||
| req: Request, | ||
| res: Response, | ||
| next: NextFunction | ||
| ) => { | ||
| const membership_id = req.currentUser?.pi_uid; | ||
|
|
||
| try { | ||
| logger.info(`Checking if membership exists for user ID: ${membership_id}`); | ||
| const currentMembership: IMembership | null = await Membership.findOne({pi_uid: membership_id}); | ||
|
|
||
| if (currentMembership) { | ||
| req.currentMembership = currentMembership; | ||
| logger.info(`Membership found: ${currentMembership._id}`); | ||
| return next(); | ||
| } else { | ||
| logger.warn(`Membership not found for user ID: ${membership_id}`); | ||
| return res.status(404).json({message: "Membership not found"}); | ||
| } | ||
| } catch (error) { | ||
| logger.error('Failed to identify membership:', error); | ||
| res.status(500).json({ message: 'Failed to identify | membership not found; please try again later'}); | ||
| } | ||
| }; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your version is clean, but I noticed updateOrRenewMembership lacks input validation to guard against malformed payloads. Do you handle this somewhere else? Consider adding:
if (
!membership_class ||
!Object.values(MembershipClassType).includes(membership_class) ||
typeof membership_duration !== 'number' ||
typeof mappi_allowance !== 'number'
) {
return res.status(400).json({ error: 'Invalid request payload' });
}
We might consider using Zod or similar down the road.