-
Notifications
You must be signed in to change notification settings - Fork 1
Top events updated #149
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
Top events updated #149
Changes from all commits
fa783ec
b747c66
a690c86
bbeddfd
e7f309d
81105d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -40,6 +40,38 @@ public ResponseEntity<List<Event>> getAllEvents() { | |||||||||
| return ResponseEntity.ok(events); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| @GetMapping("/top") | ||||||||||
| public ResponseEntity<List<java.util.Map<String, Object>>> getTopEvents() { | ||||||||||
|
||||||||||
| // Get all events | ||||||||||
| List<Event> allEvents = eventRepository.findAll(); | ||||||||||
|
||||||||||
| List<Event> allEvents = eventRepository.findAll(); | |
| List<Event> allEvents = eventRepository.findAll().stream() | |
| .filter(event -> "approved".equals(event.getStatus())) | |
| .collect(java.util.stream.Collectors.toList()); |
Copilot
AI
Nov 18, 2025
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.
Potential N+1 query problem: Calling event.getTickets().size() for each event will trigger a separate query per event since tickets is lazily loaded. This could cause significant performance issues when there are many events.
Consider using a custom query with JOIN FETCH or a database-level COUNT to efficiently retrieve ticket counts:
@Query("SELECT e FROM Event e LEFT JOIN FETCH e.tickets")
List<Event> findAllWithTickets();Or create a DTO projection query that counts tickets at the database level:
@Query("SELECT e.eventId as eventId, e.title as title, ..., COUNT(t) as ticketCount FROM Event e LEFT JOIN e.tickets t GROUP BY e.eventId")
List<EventWithTicketCount> findAllEventsWithTicketCount();
Copilot
AI
Nov 18, 2025
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.
[nitpick] The hardcoded limit of 5 events is a magic number that should be extracted as a named constant for better maintainability. If the requirement changes (e.g., to show top 10 events), the constant name makes it clear what needs to be updated.
Consider defining it at the class level:
private static final int TOP_EVENTS_LIMIT = 5;Then use:
.limit(TOP_EVENTS_LIMIT)
Copilot
AI
Nov 18, 2025
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.
The /events/top endpoint is not included in the Spring Security configuration. This endpoint should be added to the permitAll() list in SecurityConfig to allow unauthenticated access, similar to other GET event endpoints.
Add /api/events/top to line 62:
.requestMatchers(HttpMethod.GET, "/api/events", "/api/events/{id}", "/api/events/search", "/api/events/filter", "/api/events/top").permitAll()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.
Could you please look into this @daniel-buta ? Try accessing the homepage without being logged in and tell me if you can still see the top events please.
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.
Yes, I can see the top events when accessing the homepage without being logged in
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.
You can see the top events without being logged in but when you try to save an event or buy a ticket, it will redirect you to log in.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,19 @@ | ||||||||||||||||||||||||||||||||||||||
| // src/App.tsx | ||||||||||||||||||||||||||||||||||||||
| import {Routes, Route, useNavigate, Outlet, Navigate} from 'react-router-dom'; | ||||||||||||||||||||||||||||||||||||||
| import { useAuth } from './contexts/AuthContext'; | ||||||||||||||||||||||||||||||||||||||
| import { Toolbar, Box, Typography } from "@mui/material"; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import { useState, useEffect } from 'react'; | ||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||
| Toolbar, | ||||||||||||||||||||||||||||||||||||||
| Box, | ||||||||||||||||||||||||||||||||||||||
| Typography, | ||||||||||||||||||||||||||||||||||||||
| Card, | ||||||||||||||||||||||||||||||||||||||
| CardMedia, | ||||||||||||||||||||||||||||||||||||||
| CardContent, | ||||||||||||||||||||||||||||||||||||||
| CardActions, | ||||||||||||||||||||||||||||||||||||||
| Button, | ||||||||||||||||||||||||||||||||||||||
| CircularProgress, | ||||||||||||||||||||||||||||||||||||||
| } from "@mui/material"; | ||||||||||||||||||||||||||||||||||||||
| //import '@fontsource-variable/cabin'; | ||||||||||||||||||||||||||||||||||||||
| import './App.css'; | ||||||||||||||||||||||||||||||||||||||
| import SignUp from './SignUp'; | ||||||||||||||||||||||||||||||||||||||
| import Login from './Login'; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -20,6 +31,10 @@ import MyEventsPage from "./pages/MyEventsPage.tsx"; | |||||||||||||||||||||||||||||||||||||
| import EditEventPage from "./pages/EditEventPage.tsx"; | ||||||||||||||||||||||||||||||||||||||
| import ScanTicketPage from "./pages/ScanTicketPage.tsx"; | ||||||||||||||||||||||||||||||||||||||
| import AdminDashboard from "./pages/AdminDashboard.tsx"; | ||||||||||||||||||||||||||||||||||||||
| import { getTopEvents } from './api/events.api'; | ||||||||||||||||||||||||||||||||||||||
| import { saveEvent, checkIfSaved } from './api/savedEvents.api'; | ||||||||||||||||||||||||||||||||||||||
| import type { Event } from './types/event.interface'; | ||||||||||||||||||||||||||||||||||||||
| import { useSnackbar } from 'notistack'; | ||||||||||||||||||||||||||||||||||||||
| import ApproveEventsPage from "./pages/ApproveEventsPage.tsx"; | ||||||||||||||||||||||||||||||||||||||
| import EmailVerificationPage from "./pages/EmailVerificationPage.tsx"; | ||||||||||||||||||||||||||||||||||||||
| import TwoFactorAuthPage from "./pages/TwoFactorAuthPage.tsx"; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -47,6 +62,81 @@ function BlankLayout() { | |||||||||||||||||||||||||||||||||||||
| function Home() { | ||||||||||||||||||||||||||||||||||||||
| const navigate = useNavigate(); | ||||||||||||||||||||||||||||||||||||||
| const { user } = useAuth(); | ||||||||||||||||||||||||||||||||||||||
| const { enqueueSnackbar } = useSnackbar(); | ||||||||||||||||||||||||||||||||||||||
| const [topEvents, setTopEvents] = useState<Event[]>([]); | ||||||||||||||||||||||||||||||||||||||
| const [loading, setLoading] = useState(true); | ||||||||||||||||||||||||||||||||||||||
| const [savedEventIds, setSavedEventIds] = useState<Set<number>>(new Set()); | ||||||||||||||||||||||||||||||||||||||
| const [savingEventId, setSavingEventId] = useState<number | null>(null); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||
| const fetchTopEvents = async () => { | ||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||
| const events = await getTopEvents(); | ||||||||||||||||||||||||||||||||||||||
| setTopEvents(events); | ||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||
| console.error('Failed to fetch top events:', error); | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
| console.error('Failed to fetch top events:', error); | |
| console.error('Failed to fetch top events:', error); | |
| enqueueSnackbar('Failed to load top events', { variant: 'error' }); |
Copilot
AI
Nov 18, 2025
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.
The N+1 loop pattern here can cause performance issues when checking many events. Each call to checkIfSaved() makes a separate API request, resulting in up to 5 sequential requests when displaying top events.
Consider batching this operation by:
- Creating a new backend endpoint that accepts multiple event IDs and returns which ones are saved, or
- Making the API calls in parallel using
Promise.all():
const savedIds = new Set<number>();
const results = await Promise.all(
topEvents.map(event =>
checkIfSaved(event.eventID).catch(() => false)
)
);
topEvents.forEach((event, index) => {
if (results[index]) savedIds.add(event.eventID);
});
setSavedEventIds(savedIds);| for (const event of topEvents) { | |
| try { | |
| const isSaved = await checkIfSaved(event.eventID); | |
| if (isSaved) { | |
| savedIds.add(event.eventID); | |
| } | |
| } catch (error) { | |
| // Ignore errors for individual checks | |
| } | |
| } | |
| const results = await Promise.all( | |
| topEvents.map(event => | |
| checkIfSaved(event.eventID).catch(() => false) | |
| ) | |
| ); | |
| topEvents.forEach((event, index) => { | |
| if (results[index]) savedIds.add(event.eventID); | |
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,3 +76,24 @@ export const updateEvent = async (eventId: number, eventData: any) => { | |
| }); | ||
| return response.data; | ||
| }; | ||
|
|
||
| export const getTopEvents = async (): Promise<Event[]> => { | ||
| const response = await axiosInstance.get('/events/top'); | ||
|
|
||
| // Transform backend data to match frontend interface | ||
| const transformedEvents: Event[] = response.data.map((event: any) => ({ | ||
| eventID: event.eventId, | ||
| title: event.title, | ||
| description: event.description, | ||
| category: event.eventType, | ||
| image: event.imageUrl ? (event.imageUrl.startsWith('http') || event.imageUrl.startsWith('https') ? [event.imageUrl] : [`http://localhost:8080${event.imageUrl}`]) : [], | ||
|
||
| price: event.price || 0, | ||
| startDate: new Date(event.startDateTime), | ||
| endDate: new Date(event.endDateTime), | ||
| location: event.location, | ||
| capacity: event.capacity, | ||
| ticketsSold: event.ticketsSold || 0, | ||
| })); | ||
|
|
||
| return transformedEvents; | ||
| }; | ||
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.
Missing JavaDoc documentation for this new public endpoint. Other endpoints in this controller (e.g., lines 82-85) have documentation describing their purpose and behavior.
Add documentation following the existing pattern: