Skip to content

Enhancement/#205 implement bookmark logic tests#772

Merged
ivilarop merged 7 commits intodevelopfrom
enhancement/#205-implement-bookmark-logic-tests
Mar 5, 2026
Merged

Enhancement/#205 implement bookmark logic tests#772
ivilarop merged 7 commits intodevelopfrom
enhancement/#205-implement-bookmark-logic-tests

Conversation

@Carlos-Martorell
Copy link
Collaborator

@Carlos-Martorell Carlos-Martorell commented Feb 22, 2026

📍 Context

Following the UI implementation in PR#203, this PR implements the full bookmark toggle logic and comprehensive test coverage for the challenge card component.

The bookmark feature allows users to save challenges for later reference. Backend endpoints (addBookmark, removeBookmark) exist in ChallengeService and are integrated in this implementation.

✅ Changes

  • Implemented toggleBookmark() method with full logic:
    • Checks user authentication before allowing bookmark actions
    • Calls ChallengeService.addBookmark() / removeBookmark() endpoints
    • Updates local state (isBookmarked, bookmarks_count) based on API response
    • Handles errors gracefully with console logging
    • Closes tooltip on click to prevent UI glitch
  • Added comprehensive unit tests:
    • Add bookmark when not bookmarked
    • Remove bookmark when already bookmarked
    • Error handling for both operations
    • Tooltip closing behavior
    • Authentication check validation
  • Integrated NgbTooltip reference to close tooltip programmatically after click

⚠️ Backend Dependency & Testing Notes

Backend Status

The bookmark functionality requires backend endpoints that are currently unstable:

  • POST /challenge/challenges/{id}/bookmarks → Returns 500 Internal Server Error
  • DELETE /challenge/challenges/{id}/bookmarks → Returns 500 Internal Server Error

Example error from console:
image

Development Workarounds

For local testing and demonstration:

  1. User authentication check temporarily disabled (commented out in code) to test UI flow
  2. Mock data used for initial bookmark states via getUserBookmarks()
  3. Service methods mocked temporarily with of({}).pipe(delay(200)) to simulate successful responses
  4. Error handling verified through actual backend error responses

Code Status

Component logic is production-ready and will work correctly once backend endpoints are stable
All unit tests pass (using mocked service responses)

🧪 Testing

Unit Tests

All tests passing ✅

  • toggleBookmark: should call addBookmark when not bookmarked
  • toggleBookmark: should call removeBookmark when already bookmarked
  • toggleBookmark: should handle error on addBookmark
  • toggleBookmark: should handle error on removeBookmark
  • toggleBookmark: should close tooltip when provided
  • toggleBookmark: should not execute if user is not logged in

Manual Testing

Tested locally with mocked service responses:

  • Click bookmark → icon changes from empty to filled ✅
  • Click again → icon changes back to empty ✅
  • Tooltip closes on click (no sticky tooltip bug) ✅
  • State persists correctly across interactions ✅

📂 Files Changed

  • src/app/shared/components/challenge-card/challenge-card.component.html — Added #bookmarkTooltip template reference
  • src/app/shared/components/challenge-card/challenge-card.component.ts — Implemented full toggleBookmark() logic with tooltip fix
  • src/app/shared/components/challenge-card/challenge-card.component.spec.ts — Added 6 comprehensive tests for bookmark functionality
  • package.json — Version bump
  • src/environments/environment.CI.dev.ts — Version bump
  • CHANGELOG.md — Added changelog entry

✅ Acceptance Criteria

  • Click on bookmark button adds/removes bookmark via service
  • isBookmarked state updates correctly after API response
  • bookmarks_count increments/decrements appropriately
  • Button icon changes between empty and filled states
  • Click event does not trigger card navigation (stopPropagation works)
  • Tooltip closes on click (no sticky tooltip bug)
  • Non-authenticated users cannot bookmark (validation implemented)
  • Errors are logged to console without breaking UX
  • Unit tests cover all scenarios: add, remove, errors, tooltip, auth
  • All existing tests continue to pass

🚫 Out of Scope

  • Backend endpoint fixes (tracked separately by backend team)
  • Bookmark count display in UI (not in current design spec)
  • Syncing bookmarks across multiple sessions/devices

Copy link
Collaborator

@Arnau-66 Arnau-66 left a comment

Choose a reason for hiding this comment

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

Reviewed: bookmark toggle logic is correctly implemented in the challenge card.
stopPropagation prevents unwanted navigation, tooltip handling is properly managed, and auth checks are in place.
State updates (isBookmarked and bookmarks_count) look consistent, and the added unit tests cover add/remove/error scenarios well.

Styles applied correctly

Image

@vaniaferreresteban
Copy link
Collaborator

I think the version should be considered as a minor instead of a minor plus patch, it introduces something new to the user ( even if it's not fully implemented)

@vaniaferreresteban
Copy link
Collaborator

This is not an async test so it shouldn't need a set time out
imatge

@vaniaferreresteban
Copy link
Collaborator

Overall i think it's well adressed and it should pass with those tweaks, good job

@Carlos-Martorell
Copy link
Collaborator Author

Hi Vania! 👋

The addBookmark and removeBookmark tests are async because they call methods that return Observables (of({})). The setTimeout ensures the Observable's subscribe callback completes before running the assertions.

Without setTimeout, the test would finish before the next callback executes, causing the assertions to fail (state wouldn't be updated yet).

The only synchronous test is "should not execute if user is not logged in" — that one doesn't call the service at all (early return), so it doesn't need setTimeout.

Let me know if you'd like me to refactor these tests differently! 🙂

@Carlos-Martorell Carlos-Martorell force-pushed the enhancement/#205-implement-bookmark-logic-tests branch from b93fb5b to 7b379a9 Compare February 25, 2026 11:27
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 5, 2026

@ivilarop ivilarop merged commit 16b6039 into develop Mar 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FE] Implement bookmark toggle logic and tests

5 participants