Feat/auth and save story #25
Conversation
hashirjamal
commented
Sep 23, 2025
hashirjamal
commented
- Login
- Signup
- Save Story
- Unsave Story
- Snack Bar
- Save Limit
There was a problem hiding this comment.
Pull Request Overview
This PR implements authentication functionality and story saving features for the Kidlytics app. The changes enable users to sign up, sign in, save stories to their personal collection, and manage their saved stories.
- Authentication with email/password and Google sign-in
- Story saving and unsaving with a 5-story limit per user
- User interface for managing saved stories with confirmation dialogs
Reviewed Changes
Copilot reviewed 16 out of 19 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/constants/limits.constants.ts | Defines save story limit constant |
| src/app/services/save.story.service.ts | Implements story save/unsave logic with limit checking |
| src/app/services/auth.store.ts | State management for authentication using signals |
| src/app/services/auth.service.ts | Firebase authentication service methods |
| src/app/header/header.ts | Updates header component with auth-aware navigation |
| src/app/header/header.html | Adds conditional auth buttons and saved stories link |
| src/app/components/signup/signup.ts | Signup component with form validation |
| src/app/components/signin/signin.ts | Signin component with email and Google auth |
| src/app/components/my-stories/my-stories.ts | Component for viewing and managing saved stories |
| src/app/components/display-story/display-story.ts | Adds save story functionality to story viewer |
| src/app/app.routes.ts | Adds routes for authentication and saved stories pages |
| firebase.ts | Exports Firebase auth instance |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/constants/limits.constants.ts
Outdated
| @@ -0,0 +1 @@ | |||
| const NO_OF_SAVED_STORIES_ALLOWED = 5; | |||
There was a problem hiding this comment.
The constant should be exported to be usable by other modules. Add 'export' keyword before 'const'.
| const NO_OF_SAVED_STORIES_ALLOWED = 5; | |
| export const NO_OF_SAVED_STORIES_ALLOWED = 5; |
| const snapshot = await getDocs(q); | ||
| const savedCount = snapshot.size; | ||
|
|
||
| if (savedCount >= 5) { |
There was a problem hiding this comment.
Replace magic number 5 with the constant from limits.constants.ts. Import NO_OF_SAVED_STORIES_ALLOWED and use it here for better maintainability.
| const storiesRef = collection(db, 'stories'); | ||
| const q = query(storiesRef, where('savedBy', 'array-contains', uid)); | ||
| const snapshot = await getDocs(q); | ||
| console.log(snapshot.docs); |
There was a problem hiding this comment.
Remove console.log statement from production code. This debug output should not be included in the final implementation.
| console.log(snapshot.docs); |
src/app/services/auth.store.ts
Outdated
| // Subscribe to Firebase auth changes | ||
| onAuthStateChanged(auth, (firebaseUser) => { | ||
| this._user.set(firebaseUser); | ||
| console.log('STate changed', this.isLoggedIn()); |
There was a problem hiding this comment.
Fix typo: 'STate' should be 'State'. Also consider removing this console.log statement for production code.
| console.log('STate changed', this.isLoggedIn()); |
| imports: [CommonModule, RouterLink], | ||
| }) | ||
| export class UserStoriesComponent { | ||
| userStories = signal<any[]>([]); |
There was a problem hiding this comment.
Replace 'any[]' with a proper type definition. Consider creating an interface for the story data structure to improve type safety.
| } finally { | ||
| this.loading.set(false); |
There was a problem hiding this comment.
Duplicate loading.set(false) calls. The loading state is set to false both inside the try block (line 34) and in the finally block (line 38). Remove the one from line 34 to avoid redundancy.
|
|
||
| <button | ||
| type="submit" | ||
| class="btn-primary p-3 rounded-lg text-white font-bold transition hover:scale-105 disabled:opacity-50 cursor-pointer border b-5 shadow-lg" |
There was a problem hiding this comment.
Fix CSS class typo: 'b-5' should be 'border-5' for proper border styling.
| class="btn-primary p-3 rounded-lg text-white font-bold transition hover:scale-105 disabled:opacity-50 cursor-pointer border b-5 shadow-lg" | |
| class="btn-primary p-3 rounded-lg text-white font-bold transition hover:scale-105 disabled:opacity-50 cursor-pointer border border-5 shadow-lg" |
|
|
||
| <button | ||
| type="submit" | ||
| class="btn-primary p-3 border b-2 rounded-lg text-white font-semibold transition hover:scale-105 disabled:opacity-50" |
There was a problem hiding this comment.
Fix CSS class typo: 'b-2' should be 'border-2' for proper border styling.
| class="btn-primary p-3 border b-2 rounded-lg text-white font-semibold transition hover:scale-105 disabled:opacity-50" | |
| class="btn-primary p-3 border border-2 rounded-lg text-white font-semibold transition hover:scale-105 disabled:opacity-50" |
