Skip to content

fix: code quality and safety improvements #743

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

Open
saurabhhhcodes wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: code quality and safety improvements #743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 1 addition & 60 deletions app/cypress/e2e/critical_flows.cy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,63 +3,4 @@ describe('Critical Event App Flows', () => {
cy.visit('/');
// Set mock user session using our Cypress hook
cy.window().then(win => {
win.setMockUser(
{
uid: 'student-test-uid',
displayName: 'Jane Doe',
email: 'jane.doe@uni.edu',
},
'student',
{
name: 'Jane Doe',
email: 'jane.doe@uni.edu',
branch: 'Computer Science',
year: '3rd Year',
points: 120,
},
);
});
});

it('should render the event feed home page correctly', () => {
// Verify welcome text is loaded from the mock user displayName
cy.contains('Welcome,').should('be.visible');
cy.contains('Jane Doe').should('be.visible');

// Verify search bar is visible
cy.get('input[placeholder="Search events..."]').should('be.visible');

// Verify recommendations section header is visible
cy.contains('RECOMMENDED FOR YOU').should('be.visible');
});

it('should support tab navigation to Leaderboard and Profile', () => {
// We should be able to navigate to Leaderboard using tab bar
cy.contains('Rankings').click();

// Verify Leaderboard screen is shown
cy.contains('LEADERBOARD').should('be.visible');
cy.contains('Top Contributors').should('be.visible');

// Navigate to Profile using tab bar
cy.contains('Profile').click();

// Verify Profile screen renders correctly
cy.contains('Jane Doe').should('be.visible');
cy.contains('jane.doe@uni.edu').should('be.visible');
cy.contains('Computer Science').should('be.visible');
cy.contains('3rd Year').should('be.visible');
cy.contains('Student Settings').should('be.visible');
});

it('should allow searching for events in the feed', () => {
const searchQuery = 'Hackathon';
// Type search query
cy.get('input[placeholder="Search events..."]')
.type(searchQuery)
.should('have.value', searchQuery);

// Verify close icon appears via stable testID
cy.get('[data-testid="clear-search-button"]').should('exist');
});
});
.catch(err => console.error(err))

@coderabbitai coderabbitai Bot Aug 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files | rg '(^|/)critical_flows\.cy\.js$|(^|/)AuthContext\.js$|(^|/).*[Ll]og[Ll]og|setMockUser|cy\.visit|cy\.getLogin' || true

echo "== target file outlint/section =="
if [ -f app/cypress/e2e/critical_flows.cy.js ]; then
  wc -l app/cypress/e2e/critical_flows.cy.js
  cat -n app/cypress/e2e/critical_flows.cy.js
fi

echo "== AuthContext section =="
if [ -f app/src/lib/AuthContext.js ]; then
  wc -l app/src/lib/AuthContext.js
  cat -n app/src/lib/AuthContext.js | sed -n '1,140p'
fi

echo "== usages in cypress =="
rg -n "setMockUser|mock(user|User)|cy\.visit|beforeEach|AuthContext|cy\.login|login" app/cypress app/src -S || true

echo "== diff stat =="
git diff --stat || true
git diff -- app/cypress/e2e/critical_flows.cy.js app/src/lib/AuthContext.js || true

Repository: roshankumar0036singh/Uni-Event

Length of output: 13105

Restore the mock-user setup for protected flow tests.

app/cypress/e2e/critical_flows.cy.js now visits / without calling setMockUser, so these assertions can run with no authenticated user state. Call globalThis.setMockUser(...) after cy.visit('/') or add another authenticated setup before the flow tests.

🧰 Tools
🪛 Biome (2.5.5)

[error] 6-6: Expected a statement but instead found '.catch(err => console.error(err))'.

(parse)

[error] 6-6: expected } but instead the file ends

(parse)

🤖 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 `@app/cypress/e2e/critical_flows.cy.js` at line 6, Restore authenticated
mock-user setup in the protected flow tests by calling
globalThis.setMockUser(...) after cy.visit('/') in the critical flow setup.
Ensure the mock user is configured before the flow assertions execute,
preserving the existing test sequence and error handling.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file exists and size =="
git ls-files | grep -F 'app/cypress/e2e/critical_flows.cy.js' || true
wc -l app/cypress/e2e/critical_flows.cy.js

echo "== relevant file contents =="
cat -n app/cypress/e2e/critical_flows.cy.js

echo "== search for mock user/window context =="
rg -n "setMockUser|beforeEach|describe|cy\.window|window" app/cypress/e2e app -S || true

Repository: roshankumar0036singh/Uni-Event

Length of output: 7621

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== JS parse/check for critical_flows =="
node - <<'JS'
const fs = require('fs');
const path = 'app/cypress/e2e/critical_flows.cy.js';
const src = fs.readFileSync(path, 'utf8');
try {
  new Function(src);
  console.log('node parses: ok');
} catch (err) {
  console.log(`node parse error: ${err.name}: ${err.message}`);
  console.log(err.stack.split('\n').slice(0, 3).join('\n'));
}
JS

echo "== package/cypress tooling availability =="
node - <<'JS'
for (const mod of ['cypress']) {
  try {
    const p = require.resolve(mod);
    console.log(`${mod}: ${p}`);
  } catch (err) {
    console.log(`${mod}: not resolvable`);
  }
}
JS

echo "== Cypress package files mentioning catch handling / .then signatures =="
fd -a 'package.json|CHANGELOG.md|dist|src' app 2>/dev/null | sed 's#^\./##' | head -50 || true

Repository: roshankumar0036singh/Uni-Event

Length of output: 551

Fix the malformed Cypress chain before merging.

Line 5 opens a .then(win => { callback, but line 6 starts with .catch(...) instead of using catch inside that callback. The beforeEach and describe blocks are also not closed, so this spec cannot be parsed.

Complete or remove the cy.window() setup, then close beforeEach and describe; .catch is not a command-chain API and should not attach here if command failures are being handled.

🧰 Tools
🪛 Biome (2.5.5)

[error] 6-6: Expected a statement but instead found '.catch(err => console.error(err))'.

(parse)

[error] 6-6: expected } but instead the file ends

(parse)

🤖 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 `@app/cypress/e2e/critical_flows.cy.js` at line 6, Fix the cy.window() setup in
the beforeEach hook by completing or removing the .then(win => { callback; do
not attach .catch(...) as a Cypress command-chain API. Ensure the beforeEach and
describe blocks are properly closed so the spec parses successfully.

Source: Linters/SAST tools

6 changes: 4 additions & 2 deletions app/src/lib/CalendarService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ export const createMeetEvent = async (accessToken, eventDetails) => {
},
);

const data = await response.json();
if (!response.ok) throw new Error("Request failed");
const data = await response.json();
Comment on lines +87 to +88

@coderabbitai coderabbitai Bot Aug 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files | rg 'app/src/lib/(CalendarService\.js|__tests__/CalendarService\.test\.js)$' || true

echo "== CalendarService outline =="
ast-grep outline app/src/lib/CalendarService.js || true

echo "== CalendarService relevant lines =="
sed -n '1,140p' app/src/lib/CalendarService.js

echo "== CalendarService tests relevant lines =="
sed -n '1,220p' app/src/lib/__tests__/CalendarService.test.js

echo "== fetch resolvers in tests =="
python3 - <<'PY'
from pathlib import Path
p=Path('app/src/lib/__tests__/CalendarService.test.js')
s=p.read_text()
for i,line in enumerate(s.splitlines(),1):
    if 'mockResolvedValueOnce' in line or 'mockRejectedValueOnce' in line:
        print(f"{i}: {line}")
PY

Repository: roshankumar0036singh/Uni-Event

Length of output: 8698

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== deterministic probe for response.ok guard behavior =="
python3 - <<'PY'
def read_error_mock_without_ok(body_error='API Failure'):
    response = {
        "ok": None,
        "status": None,
        "json": lambda: {"error": body_error}
    }
    try:
        if not response["ok"]:
            raise Exception("Request failed")
        data = response["json"]()
        return ("parsed", data)
    except Exception as e:
        return ("raised", str(e))

for body in ["API Failure", "Calendar Failure"]:
    print({body: read_error_mock_without_ok(body)})
PY

Repository: roshankumar0036singh/Uni-Event

Length of output: 322

Update the calendar response mocks for the new response.ok guard.

Both failing calendar tests return mock responses without ok, and both functions now check if (!response.ok) before parsing response bodies. As written, these tests throw "Request failed" instead of "API Failure" or "Calendar Failure". Add ok: true to the body-error mocks and add a separate ok: false HTTP-failure path.

🤖 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 `@app/src/lib/CalendarService.js` around lines 87 - 88, Update the calendar
response mocks used by the affected tests to include ok: true for body-error
scenarios, preserving their expected “API Failure” and “Calendar Failure”
results. Add a separate mock with ok: false to cover the HTTP-failure branch
introduced by the response.ok guard in the calendar request functions.


if (data.error) throw new Error(data.error.message);

Expand Down Expand Up @@ -125,7 +126,8 @@ export const addToCalendar = async (accessToken, event) => {
},
);

const data = await response.json();
if (!response.ok) throw new Error("Request failed");
const data = await response.json();
if (data.error) throw new Error(data.error.message);
return data;
} catch (error) {
Expand Down
22 changes: 1 addition & 21 deletions app/src/lib/usePushNotifications.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,24 +52,4 @@ export const usePushNotifications = () => {

useEffect(() => {
registerForPushNotificationsAsync().then(token => setExpoPushToken(token));

notificationListener.current = Notifications.addNotificationReceivedListener(
notification => {
setNotification(notification);
},
);

responseListener.current = Notifications.addNotificationResponseReceivedListener(
response => {
logger.debug(response);
},
);

return () => {
Notifications.removeNotificationSubscription(notificationListener.current);
Notifications.removeNotificationSubscription(responseListener.current);
};
}, []);

return { expoPushToken, notification };
};
.catch(err => console.error(err))

@coderabbitai coderabbitai Bot Aug 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate files =="
git ls-files | rg '(^|/)usePushNotifications\.js$|(^|/)AttendanceDashboard\.js$' || true

echo "== Show relevant slices with line numbers =="
for f in app/src/lib/usePushNotifications.js app/src/screens/AttendanceDashboard.js; do
  if [ -f "$f" ]; then
    echo "--- $f ($(wc -l < "$f") lines) ---"
    sed -n '1,120p' "$f" | cat -n
  else
    echo "missing $f"
  fi
done

echo "== Parse check with Node parser (read-only) =="
python3 - <<'PY'
from pathlib import Path
files = Path('app/src/lib/usePushNotifications.js').resolve(), Path('app/src/screens/AttendanceDashboard.js').resolve()
for p in files:
    if p.exists():
        print(p)
        print(open(p, encoding='utf-8').read()[-1200:])
PY

Repository: roshankumar0036singh/Uni-Event

Length of output: 10110

🌐 Web query:

AttendanceDashboard.js 167 getDoc doc(db COLLECTIONS.EVENTS eventId) then catch

💡 Result:

In the Firebase Modular SDK (v9+), the getDoc function returns a Promise that resolves to a DocumentSnapshot [1]. You can handle this operation using either the.then.catch pattern or async/await syntax [2][3]. Based on your query regarding AttendanceDashboard.js, the following implementation shows how to use getDoc with a promise chain and error handling: import { doc, getDoc } from "firebase/firestore"; // Assuming db and COLLECTIONS.EVENTS are defined correctly const docRef = doc(db, COLLECTIONS.EVENTS, eventId); getDoc(docRef).then((docSnap) => { if (docSnap.exists) { console.log("Document data:", docSnap.data); } else { console.log("No such document!"); } }).catch((error) => { console.error("Error getting document:", error); }); Key points for this implementation: 1. Document Reference: Use the doc function to create a reference to the specific document, passing the database instance, collection name, and document ID [2][4]. 2. Fetching: getDoc is an asynchronous operation that returns a Promise [1]. 3. Existence Check: Always check if the document exists using the.exists method on the returned snapshot before attempting to access its data with.data [2][5][6]. 4. Error Handling: The.catch block will capture errors related to the fetch operation, such as permission issues or network failures [2][3]. Alternatively, many developers prefer async/await for cleaner syntax: try { const docSnap = await getDoc(docRef); if (docSnap.exists) { console.log("Document data:", docSnap.data); } else { console.log("No such document!"); } } catch (error) { console.error("Error getting document:", error); }

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== AttendanceDashboard remaining lines =="
sed -n '140,180p' app/src/screens/AttendanceDashboard.js | cat -n

echo "== usePushNotifications remaining lines =="
sed -n '50,70p' app/src/lib/usePushNotifications.js | cat -n

echo "== Deterministic JS parse probe for dangling catch syntax =="
node - <<'JS'
const snippet = `
registerForPushNotificationsAsync().then(token => setExpoPushToken(token));
.catch(err => console.error(err))
`;
try {
  Function(snippet);
  console.log("Parses as", true);
} catch (err) {
  console.log("Parses as", false, err.message);
}
JS

Repository: roshankumar0036singh/Uni-Event

Length of output: 1732

Complete the broken promise chains and closures.

Both changed files stop before the useEffect, hook, or component closing braces, and each leaves .catch as an invalid standalone statement.

  • app/src/lib/usePushNotifications.js#L55: attach .catch to the registration promise, close useEffect, and close usePushNotifications.
  • app/src/screens/AttendanceDashboard.js#L167: close the event-document then, then attach .catch, close useEffect, and close the component.
🧰 Tools
🪛 Biome (2.5.5)

[error] 55-55: Expected a statement but instead found '.catch(err => console.error(err))'.

(parse)

[error] 55-55: expected } but instead the file ends

(parse)

📍 Affects 2 files
  • app/src/lib/usePushNotifications.js#L55-L55 (this comment)
  • app/src/screens/AttendanceDashboard.js#L167-L167
🤖 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 `@app/src/lib/usePushNotifications.js` at line 55, Complete the promise chains
and closing braces at app/src/lib/usePushNotifications.js:55 by attaching catch
to the registration promise, then closing useEffect and usePushNotifications; at
app/src/screens/AttendanceDashboard.js:167, close the event-document then
callback, attach catch to the promise chain, and close useEffect and the
component.

Source: Linters/SAST tools

Loading