feat: Add new Endpoint + Screen#70
Conversation
Works for both web and mobile
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR introduces a company-type filtering feature across the full stack. The backend adds a repository query method, service layer logic, and controller endpoint for retrieving companies by type while updating the CompanyType enum. The frontend adds a new CompanyList screen, useCompanies hook, company card components, and state UI (loading, error, empty). Storage abstraction replaces SecureStore, and translations and exception handling are added. ChangesCompany Type Filtering Feature
Sequence DiagramsequenceDiagram
participant User
participant HomeScreen
participant CompanyListScreen
participant useCompanies
participant companyService
participant Backend
participant Database
User->>HomeScreen: Select vehicle type
HomeScreen->>CompanyListScreen: Navigate with vehicleType param
CompanyListScreen->>useCompanies: Hook initializes with vehicleType
useCompanies->>useCompanies: useEffect triggered by vehicleType
useCompanies->>companyService: getCompaniesByType(vehicleType)
companyService->>Backend: GET /api/companies/type/{vehicleType}
Backend->>Database: findByCompanyType(CompanyType)
Database-->>Backend: Company entities
Backend-->>companyService: List<CompanyResponseDTO>
companyService-->>useCompanies: Company[]
useCompanies->>useCompanies: setState(companies, loading=false)
useCompanies-->>CompanyListScreen: { companies, loading, error, reload }
CompanyListScreen->>CompanyListScreen: Body renders company cards
Body->>Body: Render CompanyListHeader + CompanyCard list
User->>CompanyCard: Click call button
CompanyCard->>User: Initiate phone call with company phone
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
frontend/src/components/company/CompanyCard.tsx (1)
13-15: ⚡ Quick winMemoize
createStyles(theme)to avoid recreating style objects on every list-item render.
createStyles(theme)is called unconditionally on every render ofCompanyCard. Since this component is rendered for each item in a list, each re-render creates new style objects unnecessarily. Wrapping withuseMemoensures styles are only recomputed whenthemechanges. The same pattern applies toErrorState,EmptyState, andLoadingState, though it matters most here in the list context.♻️ Proposed refactor
-import React from 'react'; +import React, { useMemo } from 'react'; import { View, Text, Pressable } from 'react-native'; import { MaterialIcons } from '@expo/vector-icons'; import { Company } from '../../types/company'; import { useTheme } from '../../hooks/useTheme'; import { createStyles } from './Body.style'; const CompanyCard = ({ company, onCall }: CompanyCardProps) => { const { theme } = useTheme(); - const styles = createStyles(theme); + const styles = useMemo(() => createStyles(theme), [theme]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/company/CompanyCard.tsx` around lines 13 - 15, CompanyCard currently calls createStyles(theme) on every render causing new style objects per list item; wrap the call in useMemo so styles = useMemo(() => createStyles(theme), [theme]) to only recompute when theme changes; apply the same pattern to ErrorState, EmptyState, and LoadingState where createStyles(theme) is used to avoid unnecessary re-creation of style objects in list renders.frontend/src/components/company/Body.style.tsx (1)
94-140: ⚡ Quick winHardcoded colors bypass the theme, breaking dark/light mode consistency.
#FEF3C7,#92400E,#10B981, and#FFFFFFare hardcoded rather than referencingtheme.colors.*. When the app switches between dark and light themes, these elements will stay fixed and contrast ratios can break. Move these values into the theme palette (or at minimum into named constants) so they respond to theme changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/company/Body.style.tsx` around lines 94 - 140, The styles in Body.style.tsx use hardcoded colors in ratingContainer, ratingText, callButton (and any other hex literals like '#FFFFFF') which bypass the theme; update these style objects to reference theme color tokens instead (e.g., theme.colors.ratingBackground, theme.colors.ratingText, theme.colors.callButtonBackground, theme.colors.background or textPrimary) and update shadowColor to use a theme token or conditional to match dark/light modes; then add the corresponding named color keys to your theme palette (or central constants) with light/dark variants so the UI follows theme changes consistently.frontend/src/hooks/useCompanies.ts (1)
12-28: ⚡ Quick winWrap
loadCompaniesinuseCallbackto satisfy exhaustive deps and prevent stale closures.
loadCompaniesis called insideuseEffectbut is absent from the dependency array. While the current code is functionally correct (becausevehicleTypeis in deps), it violates thereact-hooks/exhaustive-depsrule and leaves a latent stale-closure risk. As a side effect, rapidvehicleTypechanges (e.g., back-and-forth navigation) can leave concurrent fetches that settle out of order and overwrite the correct state, since there is no cleanup / abort.♻️ Proposed refactor
-import { useState, useEffect } from 'react'; +import { useState, useEffect, useCallback } from 'react'; import { VehicleType } from '../types/vehicle'; import { Company } from '../types/company'; import { getCompaniesByType } from '../services/companyService'; import { parseApiError } from '../utils/errorParser'; export const useCompanies = (vehicleType: VehicleType) => { const [companies, setCompanies] = useState<Company[]>([]); const [loading, setLoading] = useState(true); const [error, setError] = useState(''); + const loadCompanies = useCallback(async () => { + setLoading(true); + setError(''); + try { + const data = await getCompaniesByType(vehicleType); + setCompanies(data); + } catch (err: any) { + setError(parseApiError(err)); + } finally { + setLoading(false); + } + }, [vehicleType]); + useEffect(() => { loadCompanies(); - }, [vehicleType]); + }, [loadCompanies]); - - const loadCompanies = async () => { - setLoading(true); - setError(''); - - try { - const companies = await getCompaniesByType(vehicleType); - setCompanies(companies); - } catch (err: any) { - setError(parseApiError(err)); - } finally { - setLoading(false); - } - }; return { companies, loading, error, reload: loadCompanies }; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/hooks/useCompanies.ts` around lines 12 - 28, Wrap the loadCompanies function in useCallback (dependent on vehicleType) so the useEffect can include it in its dependency array and satisfy exhaustive-deps; inside loadCompanies use an AbortController or a local request token (e.g., incrementing id) to ignore out-of-order responses and ensure you call setLoading, setCompanies, and setError only for the latest request, and clean up the effect by aborting or invalidating the request in the useEffect cleanup to prevent stale closures when vehicleType changes; references: loadCompanies, useEffect, getCompaniesByType, setCompanies, setLoading, setError, parseApiError.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/main/java/com/angel/autonow/company/CompanyService.java`:
- Around line 62-64: In getAllCompaniesByCompanyType, make enum parsing more
robust by trimming input and using a locale-independent uppercase conversion
before calling CompanyType.valueOf; e.g., replace
CompanyType.valueOf(companyType.toUpperCase()) with
CompanyType.valueOf(companyType.trim().toUpperCase(Locale.ROOT)) and add the
necessary Locale import so CompanyRepository.findByCompanyType receives the
correctly normalized CompanyType.
In
`@backend/src/main/java/com/angel/autonow/exception/GlobalControllerExceptionHandler.java`:
- Around line 110-122: The handlers handleIllegalStateException and
handleInvalidDataAccessApiUsageException in GlobalControllerExceptionHandler
currently allow e.getMessage() to flow into the ProblemDetail response; change
them to return a fixed, opaque 500 message (e.g. "An internal server error
occurred") instead of exposing exception details. Keep logging the full
exception (log.error(..., e)) for diagnostics, but construct and return a
ProblemDetail that uses a generic detail string (or call a sanitized helper
instead of handle(e, ...)) so no JPA/Hibernate or internal state appears in API
responses.
In `@frontend/src/components/company/Body.style.tsx`:
- Around line 58-78: The list spacing doubles because companiesList.gap: 12 and
companyCard.marginBottom: 12 both apply; remove marginBottom from the
companyCard style and rely on companiesList.gap for vertical spacing instead
(update the companyCard object in Body.style.tsx to drop marginBottom and keep
gap-based spacing on companiesList used as FlatList contentContainerStyle).
In `@frontend/src/components/company/Body.tsx`:
- Around line 33-35: handleCallCompany currently calls Linking.openURL without
handling its returned promise which can cause unhandled rejection; update the
handleCallCompany function to await or handle the promise and catch errors from
Linking.openURL(`tel:${phoneNumber}`) (e.g., make handleCallCompany async and
use try/catch or append .catch) and in the catch log the error or surface a
user-friendly message/Toast so failures (unsupported scheme or user cancel) are
handled gracefully.
In `@frontend/src/components/company/CompanyCard.tsx`:
- Around line 52-57: The Pressable in CompanyCard (the element using
styles.callButton and MaterialIcons) lacks accessibility props; update the
Pressable where onPress={() => onCall(company.phone)} is defined to include an
accessibilityLabel (e.g., "Call {company.name}" or "Call company phone") and
accessibilityRole="button" so screen readers announce it properly; ensure the
label uses company.name or company.phone for uniqueness and keep the existing
onPress/onCall behavior unchanged.
In `@frontend/src/components/company/CompanyListHeader.tsx`:
- Around line 27-32: In CompanyListHeader, guard against vehicleInfo being
undefined before using vehicleInfo.color and vehicleInfo.icon: compute safe
fallbacks (e.g., const color = vehicleInfo?.color ?? '#ccc' and const iconName =
vehicleInfo?.icon ?? 'directions-car') and use them when building the style
backgroundColor (use `${color}20` only if color is a valid hex/rgba string) and
when passing name and color props to MaterialIcons; update references to
styles.headerIcon usage and MaterialIcons props so they consume these safe
variables (avoid concatenating undefined and avoid passing undefined as icon
name).
In `@frontend/src/components/company/EmptyState.tsx`:
- Around line 25-27: The translation call in EmptyState.tsx uses { service:
serviceName } which may be undefined and render the literal "undefined"; update
the interpolation to pass a safe fallback by using the existing serviceName
variable (e.g., serviceName ?? '') or a default like '(unknown)' when calling
t('no-companies-description', { service: ... }), ensuring the Text rendering in
the component uses the safe value; locate this in EmptyState component where t
is invoked and replace the serviceName argument with a nullish-coalesced or
defaulted value.
In `@frontend/src/services/authService.ts`:
- Around line 37-41: The token persistence errors are being swallowed when
calling storage.setItem (seen around storage.setItem('jwt', token)) so callers
receive decodeToken(token) and think login succeeded while the token wasn't
saved; update the relevant functions (where storage.setItem is used and
decodeToken(token) is returned) to surface storage failures: either rethrow the
caught error from the catch block or return a result object/flag indicating
persistence failed so the caller can handle it, and ensure getStoredToken()
behavior is unchanged; specifically modify the try/catch around storage.setItem
to not just console.warn but propagate the error (or return { token:
decodeToken(token), persisted: false }) so callers of decodeToken(token)/login
can surface the issue to the user.
In `@frontend/src/services/storage.ts`:
- Around line 6-7: The web branch currently uses localStorage
(localStorage.setItem/getItem/removeItem) which exposes JWTs to XSS; change the
web path in the storage helper to use sessionStorage instead
(sessionStorage.setItem/getItem/removeItem) to limit persistence to the tab, or
optionally implement an in-memory only option (module-level variable used by the
same functions) if you prefer no persistence across reloads; update all
occurrences of localStorage.setItem/getItem/removeItem in the file to the chosen
alternative and ensure the native branch still uses SecureStore.
In `@frontend/src/types/company.ts`:
- Line 7: The frontend company type declares phone as required but the backend
allows null; update the phone property in the interface in
frontend/src/types/company.ts (the phone field on the Company/CompanyResponse
type) to be optional and accept null (i.e., make it optional and include null in
the union) so consumers handle absent values; update usages that assume a
non-null phone to guard or render conditionally and run the TypeScript build to
verify no remaining violations.
---
Nitpick comments:
In `@frontend/src/components/company/Body.style.tsx`:
- Around line 94-140: The styles in Body.style.tsx use hardcoded colors in
ratingContainer, ratingText, callButton (and any other hex literals like
'#FFFFFF') which bypass the theme; update these style objects to reference theme
color tokens instead (e.g., theme.colors.ratingBackground,
theme.colors.ratingText, theme.colors.callButtonBackground,
theme.colors.background or textPrimary) and update shadowColor to use a theme
token or conditional to match dark/light modes; then add the corresponding named
color keys to your theme palette (or central constants) with light/dark variants
so the UI follows theme changes consistently.
In `@frontend/src/components/company/CompanyCard.tsx`:
- Around line 13-15: CompanyCard currently calls createStyles(theme) on every
render causing new style objects per list item; wrap the call in useMemo so
styles = useMemo(() => createStyles(theme), [theme]) to only recompute when
theme changes; apply the same pattern to ErrorState, EmptyState, and
LoadingState where createStyles(theme) is used to avoid unnecessary re-creation
of style objects in list renders.
In `@frontend/src/hooks/useCompanies.ts`:
- Around line 12-28: Wrap the loadCompanies function in useCallback (dependent
on vehicleType) so the useEffect can include it in its dependency array and
satisfy exhaustive-deps; inside loadCompanies use an AbortController or a local
request token (e.g., incrementing id) to ignore out-of-order responses and
ensure you call setLoading, setCompanies, and setError only for the latest
request, and clean up the effect by aborting or invalidating the request in the
useEffect cleanup to prevent stale closures when vehicleType changes;
references: loadCompanies, useEffect, getCompaniesByType, setCompanies,
setLoading, setError, parseApiError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7fe4a45b-d37a-4ea0-9480-a7485862bce2
📒 Files selected for processing (24)
backend/src/main/java/com/angel/autonow/company/CompanyController.javabackend/src/main/java/com/angel/autonow/company/CompanyRepository.javabackend/src/main/java/com/angel/autonow/company/CompanyService.javabackend/src/main/java/com/angel/autonow/company/CompanyType.javabackend/src/main/java/com/angel/autonow/exception/GlobalControllerExceptionHandler.javafrontend/src/assets/translate/bg.jsonfrontend/src/assets/translate/en.jsonfrontend/src/components/company/Body.style.tsxfrontend/src/components/company/Body.tsxfrontend/src/components/company/CompanyCard.tsxfrontend/src/components/company/CompanyListHeader.tsxfrontend/src/components/company/EmptyState.tsxfrontend/src/components/company/ErrorState.tsxfrontend/src/components/company/LoadingState.tsxfrontend/src/components/home/Body.tsxfrontend/src/hooks/useCompanies.tsfrontend/src/navigation/Navigation.tsxfrontend/src/screens/company/CompanyList.style.tsxfrontend/src/screens/company/CompanyList.tsxfrontend/src/services/ApiClient.tsxfrontend/src/services/authService.tsfrontend/src/services/companyService.tsfrontend/src/services/storage.tsfrontend/src/types/company.ts
|


Summary by CodeRabbit
Release Notes