Skip to content

did some code refactoring for inconsitent data handling #103

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.

Sign up for GitHub

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
Embers2512 merged 3 commits into from
Oct 19, 2025
Merged

did some code refactoring for inconsitent data handling #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a554968
did some code refactoring for inconsitent data handling
ntur101 Oct 19, 2025
3d9575c
fixed the test cases that were failing due to the refactor
ntur101 Oct 19, 2025
e9d9de1
some minor tweaks to fix small bugs
ntur101 Oct 19, 2025
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
132 changes: 48 additions & 84 deletions frontend/src/lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { clsx } from 'clsx';
import { normalizeItem } from '../services/firestoreNormalizer';

/**
* Combine multiple class names conditionally
Expand All @@ -12,48 +13,65 @@ export function cn(...inputs) {

// Default values for when data is missing or invalid
const DEFAULT_IMAGE_URL = '/placeholder.svg';
const DEFAULT_LOCATION = 'Unknown';
const DEFAULT_REPORTER = { name: 'Unknown', trust: false };

/**
* Map category names to consistent values for UI
* Handles variations like 'accessories' vs 'accessory', 'keys/cards' -> 'keys-cards'
* @param {string} category - Raw category string
* @returns {string} Normalized category name
*/
const normalizeCategory = (category) => {
const categoryMap = {
'accessories': 'accessory',
'keys/cards': 'keys-cards',
'wallets': 'wallet',
'documents': 'document',
'stationery': 'stationery',
'electronics': 'electronics',
'clothing': 'clothing',
'other': 'other'
};

return categoryMap[category] || category;
};

/**
* Clean up Firestore data to match our UI expectations
* Handles different field names and data formats from various sources
* REFACTORED: Now uses centralized normalizeItem from firestoreNormalizer service
* This function wraps the service to add UI-specific fields like 'reporter'
*
* @param {Object} data - Raw document data from Firestore
* @param {string} id - Document ID (optional)
* @returns {Object} Normalized item object ready for the UI
*/
export const normalizeFirestoreItem = (data, id) => {
try {
// ...existing code...
const imageUrl = data.imageURL || data.imageUrl || DEFAULT_IMAGE_URL;
const kindRaw = data.kind || data.status || '';
const kind = String(kindRaw).toLowerCase();
const typeRaw = data.category || data.type || '';
const category = normalizeCategory(String(typeRaw).toLowerCase());
// Use centralized normalization service
const normalized = normalizeItem(data, id);

if (!normalized) {
throw new Error('normalizeItem returned null');
}

// Add UI-specific fields that aren't in the base normalizer
const reporter = normalizeReporter(data.reporter, data.postedBy);
const date = normalizeDate(data.date);
const location = data.location || DEFAULT_LOCATION;
const coordinates = data.coordinates || null;

// Claim fields
const claimed = Boolean(data.claimed);
const claimedAt = data.claimedAt ? normalizeDate(data.claimedAt) : null;
const claimedBy = data.claimedBy || null;

// Apply category normalization for UI (keys/cards -> keys-cards, etc.)
const category = normalizeCategory(normalized.category);

return {
id: id || data.id || '',
kind,
...normalized,
// Override imageUrl default if it's empty
imageUrl: normalized.imageUrl || DEFAULT_IMAGE_URL,
// Override location default for UI consistency
location: normalized.location || 'Unknown',
// Apply UI-specific category normalization
category,
title: String(data.title || '').trim(),
description: String(data.description || '').trim(),
imageUrl,
date,
location,
coordinates,
// Add UI-specific fields
reporter,
claimed,
claimedAt,
claimedBy,
coordinates,
};
Comment on lines +65 to 75
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

This refactor removes the previous category normalization (e.g., 'keys/cards' -> 'keys-cards', 'accessories' -> 'accessory'), changing the API shape that UI code may rely on for filtering/styling. Either re-apply the mapping here (post-normalization) or move it into normalizeItem so that all consumers receive consistent, normalized category values.

Copilot uses AI. Check for mistakes.
Comment on lines +65 to 75
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

[nitpick] Previously this wrapper defaulted location to 'Unknown' for UI display; now missing locations will be an empty string (normalizeItem returns ''), while the error path still sets 'Unknown'. For consistent UI, consider adding location: normalized.location || 'Unknown' here.

Copilot uses AI. Check for mistakes.
} catch (error) {
console.error('Error normalizing Firestore item:', error);
Expand All @@ -66,34 +84,16 @@ export const normalizeFirestoreItem = (data, id) => {
description: 'This item could not be loaded properly.',
imageUrl: DEFAULT_IMAGE_URL,
date: new Date().toISOString(),
location: DEFAULT_LOCATION,
location: 'Unknown',
coordinates: null,
reporter: DEFAULT_REPORTER,
claimed: false,
claimedAt: null,
claimedBy: null,
};
}
};

/**
* Map category names to consistent values
* Handles variations like 'accessories' vs 'accessory'
* @param {string} category - Raw category string
* @returns {string} Normalized category name
*/
const normalizeCategory = (category) => {
const categoryMap = {
'accessories': 'accessory',
'keys/cards': 'keys-cards',
'wallets': 'wallet',
'documents': 'document',
'stationery': 'stationery',
'electronics': 'electronics',
'clothing': 'clothing',
'other': 'other'
};

return categoryMap[category] || category;
};

/**
* Extract reporter information from various data formats
* Handles both direct user objects and Firestore document references
Expand Down Expand Up @@ -125,42 +125,6 @@ const normalizeReporter = (reporter, postedBy) => {
return DEFAULT_REPORTER;
};

/**
* Convert various date formats to ISO strings
* Handles Firestore timestamps, Date objects, strings, and numbers
* @param {any} dateValue - Date value from Firestore or form
* @returns {string} ISO date string for consistent display
*/
const normalizeDate = (dateValue) => {
try {
// Handle Firestore Timestamp objects
if (dateValue?.toDate && typeof dateValue.toDate === 'function') {
return dateValue.toDate().toISOString();
} else if (dateValue instanceof Date) {
// Handle JavaScript Date objects
return dateValue.toISOString();
} else if (typeof dateValue === 'string') {
// Handle string dates
const parsed = new Date(dateValue);
if (!isNaN(parsed.getTime())) {
return parsed.toISOString();
}
} else if (typeof dateValue === 'number') {
// Handle timestamp numbers (milliseconds since epoch)
const parsed = new Date(dateValue);
if (!isNaN(parsed.getTime())) {
return parsed.toISOString();
}
}

// Fallback to current date if parsing fails
return new Date().toISOString();
} catch (error) {
console.warn('Error normalizing date:', error);
return new Date().toISOString();
}
};

/**
* Pre-built button styles to reduce duplication
* Each variant has its own color scheme and hover states
Expand Down
52 changes: 34 additions & 18 deletions frontend/src/pages/Messages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { db } from '../firebase/config';
import { ArrowLeft, Send, Check } from '../components/ui/icons';
import { Button } from '../components/ui/button';
import { cardStyles } from '../lib/utils';
import { normalizeConversation, normalizeMessage, normalizeItem } from '../services/firestoreNormalizer';

const MessagesPage = () => {
const [user, setUser] = useState(null);
Expand All @@ -31,11 +32,15 @@ const MessagesPage = () => {
const messagesEndRef = useRef(null);

// Helper: fetch item document by id (returns null if not found)
// REFACTORED: Now uses normalizeItem for consistent data shape
const fetchItemData = useCallback(async (itemId) => {
if (!itemId) return null;
try {
const itemDoc = await getDoc(doc(db, 'items', itemId));
return itemDoc.exists() ? { id: itemDoc.id, ...itemDoc.data() } : null;
if (!itemDoc.exists()) return null;

// Use normalizer for consistent data structure
return normalizeItem(itemDoc.data(), itemDoc.id);
} catch (error) {
console.error('Error fetching item:', error);
return null;
Expand All @@ -57,17 +62,22 @@ const MessagesPage = () => {
}, []);

// Helper: enrich a list of conversation docs with item data and participant name
// REFACTORED: Now uses normalizeConversation for consistent data structure
const enrichConversations = useCallback(async (docs, userUid) => {
const promises = docs.map(async (docSnapshot) => {
const conversationData = docSnapshot.data();

// Normalize the conversation data first
const normalized = normalizeConversation(conversationData, docSnapshot.id);

// Fetch related data
const [itemData, otherParticipantName] = await Promise.all([
fetchItemData(conversationData.itemId),
fetchUserNameById(conversationData.participants.find(id => id !== userUid))
fetchItemData(normalized.itemId),
fetchUserNameById(normalized.participants.find(id => id !== userUid))
]);

return {
id: docSnapshot.id,
...conversationData,
...normalized,
item: itemData,
otherParticipantName
};
Expand All @@ -90,6 +100,7 @@ const MessagesPage = () => {
}, [auth, navigate]);

// Load conversations for the current user
// REFACTORED: Uses normalizeConversation for consistent data handling
useEffect(() => {
if (!user) return;

Expand All @@ -100,10 +111,15 @@ const MessagesPage = () => {
try {
const enriched = await enrichConversations(snapshot.docs, user.uid);

// Sort conversations by lastMessageTime (handle missing timestamps)
// Sort conversations by lastMessageTime using normalized timestamps
// Fall back to createdAt if lastMessageTime is missing
enriched.sort((a, b) => {
const aTime = a.lastMessageTime?.seconds || a.createdAt?.seconds || 0;
const bTime = b.lastMessageTime?.seconds || b.createdAt?.seconds || 0;
const aTime = a.lastMessageTime
? new Date(a.lastMessageTime).getTime()
: (a.createdAt ? new Date(a.createdAt).getTime() : 0);
const bTime = b.lastMessageTime
? new Date(b.lastMessageTime).getTime()
: (b.createdAt ? new Date(b.createdAt).getTime() : 0);
return bTime - aTime;
});

Expand All @@ -124,29 +140,29 @@ const MessagesPage = () => {
}, [user, searchParams, enrichConversations]);

// Load messages for selected conversation
// REFACTORED: Uses normalizeMessage for consistent timestamp handling
useEffect(() => {
if (!selectedConversation) {
setMessages([]);
return;
}
const messagesRef = collection(db, 'messages');

// Try without ordering first to see if messages exist
const q = query(
messagesRef,
where('conversationId', '==', selectedConversation.id)
);

const unsubscribe = onSnapshot(q, (snapshot) => {
const messagesList = snapshot.docs.map(doc => ({
id: doc.id,
...doc.data()
}));
// Use normalizer for consistent message structure
const messagesList = snapshot.docs
.map(doc => normalizeMessage(doc.data(), doc.id))
.filter(msg => msg !== null);

// Sort messages by timestamp (handle cases where timestamp might be missing)
// Sort messages by timestamp (now consistently ISO strings)
messagesList.sort((a, b) => {
const aTime = a.timestamp?.seconds || 0;
const bTime = b.timestamp?.seconds || 0;
const aTime = a.timestamp ? new Date(a.timestamp).getTime() : 0;
const bTime = b.timestamp ? new Date(b.timestamp).getTime() : 0;
return aTime - bTime;
});

Expand Down Expand Up @@ -363,9 +379,9 @@ const MessagesPage = () => {
className={`text-xs mt-1 ${
message.senderId === user.uid ? '' : 'text-gray-500'
}`}
style={message.senderId === user.uid ? { color: '#FFFFFF' } : undefined}
style={message.senderId === user.uid ? { color: '#D1D5DB' } : undefined}
>
{message.timestamp?.toDate?.()?.toLocaleTimeString() || 'Just now'}
{message.timestamp ? new Date(message.timestamp).toLocaleTimeString() : 'Just now'}
</p>
</div>
</div>
Expand Down
14 changes: 12 additions & 2 deletions frontend/src/pages/ProfilePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,20 @@ import PropTypes from 'prop-types'
import { getUserPosts, formatTimestamp, updateItemStatus, updateItem } from "../firebase/firestore"
import { Card, CardContent, CardHeader, CardTitle } from "../components/ui/card"
import { ProfileBadge } from '../components/ui/ProfileBadge'
import { extractDate, normalizeTimestamp } from "../services/firestoreNormalizer"

/** ---------- Small utilities (deduplicated helpers) ---------- **/
const coalesceDate = (item) =>
item.date || item.createdAt || item.created_at || item.timestamp || item.dateCreated
// REFACTORED: Replaced coalesceDate with extractDate from firestoreNormalizer
// This provides consistent date handling across the entire app
// Falls back to normalizeTimestamp to ensure we always return an ISO string
const coalesceDate = (item) => {
const extracted = extractDate(item);
if (extracted) return extracted;

// Fallback to normalizeTimestamp to handle Firestore Timestamps
const normalized = normalizeTimestamp(item.date);
return normalized || new Date().toISOString();
}

const StatusPill = memo(({ color, text }) => (
<span className={`inline-flex items-center rounded-full px-2 py-1 text-xs font-medium ${color}`}>
Expand Down
Loading