-
-
Notifications
You must be signed in to change notification settings - Fork 122
refactor(weight-screen): improve API handling, validation, and state … #177
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,9 +8,10 @@ import { | |
| Alert, | ||
| } from 'react-native'; | ||
| import {TextInput, Button, Card, Dialog, Portal} from 'react-native-paper'; | ||
| import {BASE_URL} from '@env'; | ||
| import HeaderWithBack from '../Components/HeaderWithBack'; | ||
| import Icon from 'react-native-vector-icons/Ionicons'; | ||
| import {weightApi} from '../services/weightApi'; | ||
| import dayjs from 'dayjs'; | ||
|
|
||
| export default function WeightScreen() { | ||
| const [week, setWeek] = useState(''); | ||
|
|
@@ -20,39 +21,35 @@ export default function WeightScreen() { | |
| const [refreshing, setRefreshing] = useState(false); | ||
|
|
||
| const [editVisible, setEditVisible] = useState(false); | ||
| const [editData, setEditData] = useState(null); | ||
| const [editData, setEditData] = useState({ | ||
| id: null, | ||
| week_number: '', | ||
| weight: '', | ||
| note: '', | ||
| }); | ||
|
|
||
| const [loading, setLoading] = useState(false); | ||
|
|
||
| const fetchWeightHistory = async () => { | ||
| try { | ||
| const res = await fetch(`${BASE_URL}/weight`); | ||
| if (!res.ok) { | ||
| throw new Error('Network response was not ok'); | ||
| } | ||
| const data = await res.json(); | ||
| setHistory(data.reverse()); | ||
| setLoading(true); | ||
| const data = await weightApi.getAll(); | ||
| setHistory([...data].reverse()); | ||
| } catch (err) { | ||
| console.error('Failed to fetch weights:', err); | ||
| Alert.alert('Error', `unable to laod weight history`); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
|
|
||
| const formatLocalDate = (utcDateString) => { | ||
| const formatLocalDate = utcDateString => { | ||
| if (!utcDateString) return ''; | ||
| const dateStringWithZ = utcDateString.endsWith('Z') ? utcDateString : `${utcDateString}Z`; | ||
| const date = new Date(dateStringWithZ); | ||
| if (isNaN(date.getTime())) return 'Invalid date'; | ||
| return date.toLocaleString(undefined, { | ||
| year: 'numeric', | ||
| month: '2-digit', | ||
| day: '2-digit', | ||
| hour: '2-digit', | ||
| minute: '2-digit', | ||
| second: '2-digit', | ||
| hour12: true, | ||
| }); | ||
| const date = dayjs(utcDateString); // automatically parses ISO strings | ||
| if (!date.isValid()) return 'Invalid date'; | ||
| return date.format('YYYY-MM-DD HH:mm:ss'); | ||
| }; | ||
|
|
||
|
|
||
| useEffect(() => { | ||
| fetchWeightHistory(); | ||
| }, []); | ||
|
|
@@ -64,24 +61,47 @@ export default function WeightScreen() { | |
| }; | ||
|
|
||
| const handleSubmit = async () => { | ||
| if (!week || !weight) { | ||
| Alert.alert('Error', 'Please fill in all required fields'); | ||
| const weekNum = Number(week); | ||
| const weightNum = Number(weight); | ||
|
|
||
| if (!Number.isInteger(weekNum) || weekNum < 1 || weekNum > 40) { | ||
| Alert.alert('Invalid week', 'Week must be between 1 and 40'); | ||
| return; | ||
| } | ||
|
|
||
| if (weightNum <= 0) { | ||
| Alert.alert('Invalid weight', 'Weight must be greater than 0'); | ||
| return; | ||
| } | ||
|
Comment on lines
84
to
+96
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: fd -t f WeightScreen.jsxRepository: AOSSIE-Org/BabyNest Length of output: 100 🏁 Script executed: wc -l Frontend/src/Screens/WeightScreen.jsx 2>/dev/null || echo "File not found or path needs correction"Repository: AOSSIE-Org/BabyNest Length of output: 104 🌐 Web query:
💡 Result:
Sources: [1] [2] [3] [4] 🏁 Script executed: sed -n '60,80p' Frontend/src/Screens/WeightScreen.jsxRepository: AOSSIE-Org/BabyNest Length of output: 613 🏁 Script executed: sed -n '135,155p' Frontend/src/Screens/WeightScreen.jsxRepository: AOSSIE-Org/BabyNest Length of output: 614 Add NaN validation to reject non-numeric weight inputs in create/update operations When users enter non-numeric input, 🔧 Suggested fix- if (weightNum <= 0) {
+ if (!Number.isFinite(weightNum) || weightNum <= 0) {
Alert.alert('Invalid weight', 'Weight must be greater than 0');
return;
}Apply this fix at both locations: lines 63-75 ( 🤖 Prompt for AI Agents |
||
|
|
||
| const tempId = Date.now(); // temporary id for optimistic UI | ||
| const newEntry = { | ||
| id: tempId, | ||
| week_number: weekNum, | ||
| weight: weightNum, | ||
| note, | ||
| created_at: new Date().toISOString(), | ||
| }; | ||
|
|
||
| // Optimistic update | ||
| setHistory(prev => [newEntry, ...prev]); | ||
|
|
||
| try { | ||
| await fetch(`${BASE_URL}/weight`, { | ||
| method: 'POST', | ||
| headers: {'Content-Type': 'application/json'}, | ||
| body: JSON.stringify({week_number: week, weight, note}), | ||
| const savedEntry = await weightApi.create({ | ||
| week_number: weekNum, | ||
| weight: weightNum, | ||
| note, | ||
| }); | ||
| setWeek(''); | ||
| setWeight(''); | ||
| setNote(''); | ||
| fetchWeightHistory(); | ||
| // Replace temp entry with real id from server | ||
| setHistory(prev => prev.map(e => (e.id === tempId ? savedEntry : e))); | ||
| } catch (err) { | ||
| setHistory(prev => prev.filter(e => e.id !== tempId)); | ||
| console.error('Failed to save weight:', err); | ||
| Alert.alert('Error', 'Failed to save weight entry. Please try again.'); | ||
| } finally { | ||
| setWeek(''); | ||
| setWeight(''); | ||
| setNote(''); | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -95,12 +115,12 @@ export default function WeightScreen() { | |
| text: 'Delete', | ||
| style: 'destructive', | ||
| onPress: async () => { | ||
| const prevHistory = [...history]; | ||
| setHistory(prev => prev.filter(e => e.id !== id)); | ||
| try { | ||
| await fetch(`${BASE_URL}/weight/${id}`, { | ||
| method: 'DELETE', | ||
| }); | ||
| fetchWeightHistory(); | ||
| await weightApi.remove(id); | ||
| } catch (err) { | ||
| setHistory(prevHistory); | ||
| console.error('Failed to delete weight:', err); | ||
| Alert.alert('Error', 'Failed to delete entry. Please try again.'); | ||
| } | ||
|
|
@@ -116,28 +136,56 @@ export default function WeightScreen() { | |
| }; | ||
|
|
||
| const handleUpdate = async () => { | ||
| const weekNum = Number(editData.week_number); | ||
| const weightNum = Number(editData.weight); | ||
|
|
||
| if (!Number.isInteger(weekNum) || weekNum < 1 || weekNum > 40) { | ||
| Alert.alert('Invalid week', 'Week must be between 1 and 40'); | ||
| return; | ||
| } | ||
|
|
||
| if (weightNum <= 0) { | ||
| Alert.alert('Invalid weight', 'Weight must be greater than 0'); | ||
| return; | ||
| } | ||
|
|
||
| const prevHistory = [...history]; | ||
| setHistory(prev => | ||
| prev.map(e => | ||
| e.id === editData.id | ||
| ? {...e, week_number: weekNum, weight: weightNum, note: editData.note} | ||
| : e, | ||
| ), | ||
| ); | ||
| try { | ||
| await fetch(`${BASE_URL}/weight/${editData.id}`, { | ||
| method: 'PUT', | ||
| headers: {'Content-Type': 'application/json'}, | ||
| body: JSON.stringify({ | ||
| week_number: editData.week_number, | ||
| weight: editData.weight, | ||
| note: editData.note, | ||
| }), | ||
| await weightApi.update(editData.id, { | ||
| week_number: weekNum, | ||
| weight: weightNum, | ||
| note: editData.note, | ||
| }); | ||
| setEditVisible(false); | ||
| setEditData(null); | ||
| fetchWeightHistory(); | ||
| } catch (err) { | ||
| setHistory(prevHistory); | ||
| console.error('Failed to update weight:', err); | ||
| Alert.alert('Error', 'Failed to update entry. Please try again.'); | ||
| } finally { | ||
| setEditVisible(false); | ||
| setEditData({ | ||
| id: null, | ||
| week_number: '', | ||
| weight: '', | ||
| note: '', | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <View style={styles.container}> | ||
| <HeaderWithBack title="Weight Tracker" /> | ||
| {loading && ( | ||
| <View style={{paddingVertical: 30}}> | ||
| <Text style={{textAlign: 'center'}}>Loading weight history</Text> | ||
| </View> | ||
| )} | ||
| <ScrollView | ||
| contentContainerStyle={styles.content} | ||
| refreshControl={ | ||
|
|
@@ -156,7 +204,7 @@ export default function WeightScreen() { | |
| mode="outlined" | ||
| left={<TextInput.Icon icon="calendar" />} | ||
| style={styles.input} | ||
| /> | ||
| /> | ||
| <TextInput | ||
| label="Weight (kg)" | ||
| value={weight} | ||
|
|
@@ -179,17 +227,23 @@ export default function WeightScreen() { | |
| <Button | ||
| mode="contained" | ||
| onPress={handleSubmit} | ||
| disabled={loading} | ||
| style={styles.button} | ||
| labelStyle={{fontWeight: 'bold', color: '#fff'}}> | ||
| Save Entry | ||
| </Button> | ||
| </Card.Content> | ||
| </Card> | ||
|
|
||
| {!loading && history.length === 0 && ( | ||
| <Text style={{textAlign: 'center', marginTop: 20, color: '#777'}}> | ||
| No weight entries yet. Add your first entry above. | ||
| </Text> | ||
| )} | ||
| {/* History */} | ||
| <Text style={styles.historyTitle}>Your Weight History</Text> | ||
| {history.map((entry, index) => ( | ||
| <Card key={index} style={styles.entryCard}> | ||
| {history.map(entry => ( | ||
| <Card key={entry.id} style={styles.entryCard}> | ||
| <Card.Content> | ||
| <View style={styles.entryRowBetween}> | ||
| <View style={styles.entryRow}> | ||
|
|
@@ -222,7 +276,7 @@ export default function WeightScreen() { | |
| <Text style={styles.entryNote}>Note: {entry.note}</Text> | ||
| ) : null} | ||
| <Text style={styles.entryDate}> | ||
| {formatLocalDate(entry.created_at)} | ||
| {formatLocalDate(entry.created_at)} | ||
| </Text> | ||
| </Card.Content> | ||
| </Card> | ||
|
|
@@ -233,7 +287,7 @@ export default function WeightScreen() { | |
| <Portal> | ||
| <Dialog visible={editVisible} onDismiss={() => setEditVisible(false)}> | ||
| <Dialog.Title>Edit Entry</Dialog.Title> | ||
| <Dialog.Content > | ||
| <Dialog.Content> | ||
| <TextInput | ||
| label="Week Number" | ||
| value={editData?.week_number?.toString() || ''} | ||
|
|
@@ -292,7 +346,6 @@ const styles = StyleSheet.create({ | |
| backgroundColor: 'white', | ||
| marginBottom: 15, | ||
| borderRadius: 10, | ||
|
|
||
| }, | ||
| noteInput: { | ||
| minHeight: 100, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| import {BASE_URL} from '@env'; | ||
|
|
||
| const request = async (endpoint, options = {}, retries = 1) => { | ||
| const controller = new AbortController(); | ||
| const timeout = setTimeout(() => controller.abort(), 10000); | ||
|
|
||
| try { | ||
| const res = await fetch(`${BASE_URL}${endpoint}`, { | ||
| headers: {'Content-Type': 'application/json'}, | ||
| signal: controller.signal, | ||
| ...options, | ||
| }); | ||
|
|
||
| if (!res.ok) { | ||
| const msg = await res.text(); | ||
| throw new Error(msg || 'Request failed'); | ||
| } | ||
|
|
||
| return res.json(); | ||
|
Comment on lines
+14
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result:
Fix (client side): don’t call
|
||
| } catch (err) { | ||
| if (retries > 0) { | ||
| console.warn( | ||
| `Retrying request to ${endpoint}, attempts left: ${retries}`, | ||
| ); | ||
| return request(endpoint, options, retries - 1); | ||
| } | ||
|
Comment on lines
+20
to
+26
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard retries for non‑idempotent requests Line 20: retrying POST can create duplicate entries if the server processed the first request but the client timed out. Consider limiting retries to idempotent methods (or set retries=0 for create). 🔧 Suggested fix (limit retries to idempotent methods)-const request = async (endpoint, options = {}, retries = 1) => {
+const request = async (endpoint, options = {}, retries = 1) => {
+ const method = (options.method || 'GET').toUpperCase();
+ const canRetry = ['GET', 'PUT', 'DELETE', 'HEAD', 'OPTIONS'].includes(method);
const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), 10000);
@@
- } catch (err) {
- if (retries > 0) {
+ } catch (err) {
+ if (canRetry && retries > 0) {
console.warn(
`Retrying request to ${endpoint}, attempts left: ${retries}`,
);
return request(endpoint, options, retries - 1);
}
throw err;
} finally {
clearTimeout(timeout);
}
};Also applies to: 35-40 🤖 Prompt for AI Agents |
||
| throw err; | ||
| } finally { | ||
|
|
||
| clearTimeout(timeout); | ||
| } | ||
| }; | ||
|
|
||
| export const weightApi = { | ||
| getAll: () => request('/weight', {}, 2), | ||
| create: data => | ||
| request('/weight', { | ||
| method: 'POST', | ||
| body: JSON.stringify(data), | ||
| }), | ||
| update: (id, data) => | ||
| request(`/weight/${id}`, { | ||
| method: 'PUT', | ||
| body: JSON.stringify(data), | ||
| }), | ||
| remove: id => | ||
| request(`/weight/${id}`, { | ||
| method: 'DELETE', | ||
| }), | ||
| }; | ||
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.
Fix typo in error alert copy
Line 40: “laod” → “load”.
✏️ Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents