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

Show banner popup items #152

Merged
merged 11 commits into from
Aug 23, 2024
Merged

Show banner popup items #152

merged 11 commits into from
Aug 23, 2024

Conversation

erenfn
Copy link
Collaborator

@erenfn erenfn commented Aug 20, 2024

image

Now we can see banners and popups that are in our database at our main page for banners and popup

@erenfn erenfn requested a review from uparkalau August 20, 2024 00:01
This was linked to issues Aug 20, 2024
@erenfn erenfn requested a review from thomastepi August 22, 2024 09:23
@erenfn
Copy link
Collaborator Author

erenfn commented Aug 22, 2024

Also fixed the 'flashing' issue Gorkem mentioned. Changes to Header and Dashboard are related to that

Copy link
Contributor

@uparkalau uparkalau left a comment

Choose a reason for hiding this comment

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

Could you please next time create separate branches for the frontend and backend parts? It’s one of the fundamental principles of teamwork to make small commits . This will prevent blocking yourself and other team.

Example:

Branch 1: Feature/Banner-Controller:

Commit 1: Add getBanners method to banner.controller.js.
Commit 2: Update banner.routes.js to include new route for getBanners.

Branch 2: Feature/Popup-Controller:

Commit 1: Add getPopups method to popupController.js.
Commit 2: Update popup.routes.js to include new route for getPopups.

Branch 3: Feature/Model-Updates:

Commit 1: Add bannerText field to Banner.js.
Commit 2: Add header and content fields to Popup.js.

Branch 4: Feature/Service-Methods:

Commit 1: Add getBanners method to banner.service.js.
Commit 2: Add getPopups method to popup.service.js.

Branch 5: Feature/Frontend-Updates:

Commit 1: Update routes in App.jsx.
Commit 2: Refactor Header.jsx to include user fetching logic.

banner.controller.js and popupController.js:
Extract common logic (e.g., error handling) into helper functions to reduce code duplication.
Example:

async handleRequest(req, res, serviceMethod, errorMessage) {
  try {
    const userId = req.user.id;
    const result = await serviceMethod(userId);
    res.status(200).json(result);
  } catch (err) {
    const { statusCode, payload } = internalServerError(errorMessage, err.message);
    res.status(statusCode).json(payload);
  }
}

banner.routes.js and popup.routes.js:
Group similar routes together.
Example:

router.route('/banners')
  .get(authenticateJWT, bannerController.getBanners)
  .post(authenticateJWT, bannerController.addBanner);

banner.service.js and popup.service.js:
Use more descriptive method names and methods are single-responsibility.
Example:

async fetchBannersByUser(userId) {
  return await Banner.findAll({
    where: { createdBy: userId },
    include: [{ model: db.User, as: "creator" }],
  });
}

@erenfn erenfn merged commit 0599d28 into develop Aug 23, 2024
1 check failed
@erenfn erenfn deleted the show-banner-popup-items branch January 16, 2025 23:44
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.

Implement Popups List Implement Banners List
2 participants