Skip to content

Remove Apple Authentication and Add Name Settings Feature#28

Merged
popand merged 5 commits into
mainfrom
feature/remove-apple-authentication
Aug 21, 2025
Merged

Remove Apple Authentication and Add Name Settings Feature#28
popand merged 5 commits into
mainfrom
feature/remove-apple-authentication

Conversation

@popand

@popand popand commented Aug 21, 2025

Copy link
Copy Markdown
Owner

Summary

This PR completely removes Apple authentication from the SunshAid app and introduces a comprehensive name settings feature that personalizes the user experience.

🚫 Authentication Removal

  • Removed all Apple Sign In dependencies and authentication flows
  • Deleted AuthenticationManager, KeychainService, and authentication views
  • Updated entitlements to remove Sign in with Apple capability
  • Simplified app navigation to go directly to dashboard without auth gates
  • Replaced authenticated profile with SimpleProfileView

✨ Name Settings Feature

  • Added comprehensive Settings UI with name input capability
  • Personalized dashboard greetings based on user's name
  • Graceful fallback to generic greetings when no name is set
  • User-friendly name input sheet with focus management
  • Integrated Settings access through Profile tab

🎨 UI/UX Improvements

  • Redesigned PrivacyNoticeView with modern card-based layout
  • Updated HelpSupportView to remove duplicate App Information section
  • Consistent design language across Settings, Privacy, and Help views
  • Enhanced content with emphasis on local data storage

πŸ“§ Content Updates

  • Updated email addresses from sunshadeapp@gmail.com to sunshaidapp@gmail.com
  • Updated privacy FAQ to emphasize local data storage: "All your personal data is stored locally on your device and never leaves your device"
  • Removed duplicate sections and improved content organization

🧡 Technical Fixes

  • Fixed threading violations in UserProfile by wrapping UserDefaults operations in DispatchQueue.main.async
  • Resolved "Publishing changes from background threads" warnings in tests
  • Updated tests to handle empty default names properly

πŸ§ͺ Test Updates

  • Updated UserProfileTests to allow empty default names
  • Enhanced TimeUtilsTests to verify empty name handling
  • All tests pass without threading warnings

Test plan

  • App builds successfully without authentication dependencies
  • Navigation flows directly to dashboard without auth screens
  • Settings accessible through Profile tab
  • Name input works with proper focus and validation
  • Dashboard greetings personalize correctly with/without names
  • All UI views display with consistent design patterns
  • Tests pass without threading violations
  • Privacy and Help content reflects local storage emphasis

πŸ€– Generated with Claude Code

popand and others added 3 commits August 21, 2025 11:39
- Remove all Apple Sign-In components and dependencies
- Delete AuthenticationView, AuthenticationManager, and KeychainService
- Update MainContentView to skip authentication and go directly to dashboard
- Create SimpleProfileView for non-authenticated profile management
- Remove authentication references from DashboardView and DashboardViewModel
- Clean up entitlements by removing Sign in with Apple capability
- App now starts directly without any sign-in requirement
- All user data stored locally using UserDefaults

This change simplifies the app by removing the authentication barrier,
allowing users to start using the app immediately without signing in.

πŸ€– Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Features:
- Add name setting capability in AccountSettingsView with personalized dashboard greetings
- Create NameInputSheet component for user-friendly name entry
- Update TimeUtils to handle empty names gracefully (returns generic greetings)
- Change UserProfile default name from "User" to empty string
- Connect AccountSettingsView to SimpleProfileView for easy access

Content Updates:
- Update email addresses from sunshadeapp@gmail.com to sunshaidapp@gmail.com
- Remove duplicate App Information section from HelpSupportView (available in Settings)
- Update privacy FAQ to emphasize local data storage
- Redesign PrivacyNoticeView with improved card-based layout

Test Updates:
- Fix UserProfileTests to allow empty default names
- Update TimeUtilsTests to verify empty name handling returns generic greetings
- All TimeUtils tests passing, core functionality verified

πŸ€– Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Wrap all UserDefaults writes in DispatchQueue.main.async to ensure they happen on main thread
- Prevent recursive @published updates in skinType didSet handler
- Resolves "Publishing changes from background threads is not allowed" warnings during tests
- All UserProfile tests now pass without threading violations

πŸ€– Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@popand popand self-assigned this Aug 21, 2025
@claude

claude Bot commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Claude finished @popand's task β€”β€” View job


Pull Request Review Complete βœ…

I've completed a comprehensive review of this PR. Overall, this is a well-executed major refactor that removes Apple authentication and adds name personalization features. Here's my detailed feedback:

βœ… Strengths

Threading Fixes (Excellent Work)

  • Perfect fix in UserProfile.swift:48-104 - all UserDefaults operations properly wrapped in DispatchQueue.main.async
  • Resolves the "Publishing changes from background threads" warnings
  • Consistent pattern applied to all @Published property setters

Clean Architecture

  • Excellent separation of concerns in the new Settings UI
  • Reusable components (SettingsItem, PrivacyCard, SettingsSectionHeader)
  • Well-structured navigation flow: Profile β†’ Settings β†’ Name input

User Experience

  • Graceful fallback for empty names in TimeUtils.swift:19-34
  • Focus management in name input sheet (@FocusState on line 452)
  • Input validation with trimming (line 529)

⚠️ Issues to Address

Critical: Temperature Unit Duplication

  • AccountSettingsView.swift:535-538 defines SettingsTemperatureUnit enum
  • UserProfile.swift:4-43 has TemperatureUnit enum
  • Lines 92-93 in AccountSettingsView directly save to UserDefaults, bypassing the UserProfile model
  • Risk: Data inconsistency between the two temperature unit systems

Performance Concern: Multiple UserProfile Instances

  • MainContentView.swift:54 and AccountSettingsView.swift:5 create separate @StateObject instances
  • Should use UserProfile.shared consistently for singleton pattern
  • Risk: Settings changes might not reflect immediately across views

Input Validation Gap

  • Name input allows any string without length limits
  • Risk: Extremely long names could cause UI layout issues
  • Consider adding reasonable character limit (e.g., 50 characters)

πŸ“‹ Minor Improvements

Error Handling

  • UserProfile.swift:112 uses safe defaults for invalid enum values βœ…
  • Consider logging when fallbacks are used for debugging

Code Organization

  • AccountSettingsView.swift is quite large (540+ lines)
  • Consider extracting NameInputSheet and SkinTypeSelectionView to separate files

Memory Management

  • All @StateObject and @ObservedObject usage looks correct
  • Proper use of @Environment(\.presentationMode) for dismissal

πŸ”’ Security Assessment

Authentication Removal: Low Risk

  • Clean removal of Apple Sign In dependencies
  • No authentication-dependent features remain
  • App now functions as intended (local-only data storage)

Data Storage: Secure

  • All data stored in UserDefaults (appropriate for this use case)
  • No sensitive authentication tokens or credentials stored
  • Privacy notice correctly emphasizes local storage

πŸ§ͺ Test Coverage: Excellent

UserProfileTests.swift: Comprehensive coverage

  • Tests all enum cases, conversions, and edge cases
  • Proper state management validation
  • Tests empty name handling (line 43)

TimeUtilsTests.swift: Thorough

  • Tests personalized vs. generic greetings
  • Edge cases: empty names, special characters, long names
  • Good validation of greeting formats

🎯 Recommendations

  1. Fix Temperature Unit Duplication (High Priority)

    // In AccountSettingsView, use UserProfile's enum consistently:
    @State private var selectedTemperatureUnit: TemperatureUnit = .fahrenheit
    // Remove custom SettingsTemperatureUnit enum
  2. Standardize UserProfile Usage

    // Replace @StateObject with shared instance:
    @ObservedObject private var userProfile = UserProfile.shared
  3. Add Name Length Validation

    // In NameInputSheet.swift:
    private let maxNameLength = 50
    // Truncate if needed: String(name.prefix(maxNameLength))
  4. Extract Large View Components

    • Move NameInputSheet and SkinTypeSelectionView to separate files
    • Improves maintainability and compile times

Summary

This is a high-quality PR with excellent threading fixes and clean architecture. The main issues are architectural (enum duplication) rather than functional bugs. The code is well-tested and follows SwiftUI best practices. Address the temperature unit inconsistency and you'll have a solid implementation.

Recommendation: Approve with minor fixes πŸš€


  • Read and analyze key modified files
  • Review UserProfile threading fixes
  • Examine new Settings UI implementation
  • Check test coverage and updates
  • Evaluate security implications of auth removal
  • Assess performance considerations
  • Provide comprehensive feedback

popand and others added 2 commits August 21, 2025 13:16
- Remove duplicate SettingsTemperatureUnit enum
- Use existing TemperatureUnit enum from UserProfile consistently
- Direct binding to userProfile.temperatureUnit instead of separate state
- Remove unnecessary loadCurrentSettings() function and onAppear call
- Cleaner integration with existing data model
- Uses temperatureUnit.displayName for better subtitle formatting

Resolves duplication issue and simplifies temperature unit management.

πŸ€– Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
UserProfile Standardization:
- Replace @StateObject with @ObservedObject for UserProfile.shared across all views
- Ensures consistent usage of singleton pattern without creating new instances
- Fixed in AccountSettingsView, MainContentView, and SimpleProfileView

Name Length Validation:
- Add maxNameLength constant (50 characters) to NameInputSheet
- Implement real-time name truncation using modern onChange syntax
- Add character counter with visual feedback (changes color when approaching limit)
- Prevents names from exceeding maximum length during input
- Uses iOS 17+ onChange syntax to fix deprecation warning

Improvements:
- Better user experience with clear length constraints
- Consistent data management across all UserProfile usages
- Visual feedback for users approaching character limit

πŸ€– Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@popand popand merged commit 1d99df2 into main Aug 21, 2025
1 check passed
@popand popand deleted the feature/remove-apple-authentication branch August 21, 2025 17:22
@claude

claude Bot commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Claude finished @popand's task β€”β€” View job


Comprehensive PR Review βœ…

I've completed a thorough review of this merged PR that removes Apple authentication and adds name personalization features. Here's my detailed assessment:

PR Review Tasks

  • Analyze UserProfile.swift changes and threading fixes
  • Review AccountSettingsView.swift UI implementation
  • Examine authentication removal completeness
  • Check TimeUtils.swift personalization logic
  • Review test coverage and updates
  • Assess security implications of auth removal
  • Evaluate performance considerations
  • Provide comprehensive feedback on code quality

βœ… Strengths

Excellent Threading Fixes

  • Perfect implementation in UserProfile.swift:48-104 - all UserDefaults operations properly wrapped in DispatchQueue.main.async
  • Resolves the "Publishing changes from background threads" warnings that were causing test issues
  • Consistent pattern applied across all @Published property setters

Clean Architecture & Code Quality

  • Well-structured Settings UI with reusable components (SettingsItem, PrivacyCard, SettingsSectionHeader)
  • Proper separation of concerns between UserProfile (data model) and UI views
  • Good use of SwiftUI best practices: @StateObject, @ObservedObject, @FocusState

User Experience Design

  • Graceful fallback for empty names in TimeUtils.swift:19-34
  • Smart focus management in name input sheet (@FocusState on line 452)
  • Input validation with trimming and character limits (50 chars max)
  • Personalized greetings that update automatically via Combine publishers

Complete Authentication Removal

  • βœ… Clean removal of AuthenticationManager.swift and KeychainService.swift
  • βœ… Entitlements properly updated (removed Sign in with Apple capability)
  • βœ… No remaining authentication imports or references found in codebase
  • βœ… Navigation flow simplified to go directly to dashboard

⚠️ Issues to Address

Critical: Multiple UserProfile Instances

  • MainContentView.swift:54 creates @StateObject private var userProfile = UserProfile() instead of using shared instance
  • AccountSettingsView.swift:5 correctly uses UserProfile.shared
  • Risk: Settings changes from AccountSettingsView might not immediately reflect in other parts of the app
  • Fix: Use @ObservedObject private var userProfile = UserProfile.shared consistently

Performance: Singleton Pattern Inconsistency

  • UserProfile.shared exists (line 162) but not used everywhere
  • Multiple instances could lead to state synchronization issues
  • Recommendation: Audit all UserProfile usage for consistency

Minor: Input Validation

  • Name input allows any characters without sanitization
  • While 50-character limit exists, no validation for potentially problematic characters
  • Low priority but consider basic sanitization for special characters

πŸ§ͺ Test Coverage: Excellent

UserProfileTests.swift: Comprehensive

  • Tests all enum cases, conversions, and edge cases
  • Proper state management validation
  • Tests singleton behavior and empty name handling
  • Good coverage of threading scenarios

TimeUtilsTests.swift: Thorough

  • Tests personalized vs. generic greetings
  • Edge cases: empty names, special characters, long names
  • Good validation of greeting time ranges and formats

πŸ”’ Security Assessment: Low Risk

Authentication Removal

  • βœ… No authentication-dependent features remain
  • βœ… App correctly functions as local-only data storage
  • βœ… No sensitive tokens or credentials stored
  • βœ… Privacy notice accurately reflects local storage approach

Data Storage

  • βœ… All data stored in UserDefaults (appropriate for this use case)
  • βœ… No network communication for user data
  • βœ… Clean separation between weather data (external) and user data (local)

⚑ Performance Considerations

Positive

  • Smart caching in DashboardViewModel for weekly session data
  • Efficient Combine publishers for reactive updates
  • Proper use of @MainActor for UI updates

Potential Improvement

  • Consider lazy loading for Settings UI if it becomes complex
  • Current implementation is well-optimized for current feature set

🎯 Recommendations

High Priority

  1. Fix UserProfile Instance Usage
    // In MainContentView.swift, replace line 54:
    @ObservedObject private var userProfile = UserProfile.shared

Medium Priority

  1. Extract Large View Components

    • AccountSettingsView.swift is 540+ lines
    • Consider moving NameInputSheet and SkinTypeSelectionView to separate files
  2. Add Input Sanitization

    // In NameInputSheet, enhance validation:
    private func sanitizeName(_ input: String) -> String {
        input.trimmingCharacters(in: .whitespacesAndNewlines)
             .filter { !$0.isNewline }
    }

Low Priority

  1. Error Handling Enhancement
    • UserProfile.swift:112 has good enum fallbacks
    • Consider logging when fallbacks are used for debugging

πŸ“‹ Architecture Assessment

Strengths

  • Clear data flow: UserProfile β†’ DashboardViewModel β†’ Views
  • Proper reactive programming with Combine
  • Good separation between settings, profile data, and weather services

Patterns

  • βœ… Singleton pattern for UserProfile.shared
  • βœ… MVVM pattern with DashboardViewModel
  • βœ… Reactive updates via @Published properties

πŸ† Summary

This is a high-quality PR that successfully removes Apple authentication while adding valuable personalization features. The code demonstrates:

  • βœ… Excellent threading fixes that resolve test warnings
  • βœ… Clean architecture following SwiftUI best practices
  • βœ… Comprehensive test coverage for new functionality
  • βœ… Complete authentication removal without loose ends
  • βœ… User-friendly Settings UI with proper validation

Main Issue: Fix the UserProfile instance inconsistency in MainContentView.swift to ensure settings changes propagate correctly.

Recommendation: Approve with minor fixes πŸš€

The implementation is solid, well-tested, and ready for production with the singleton fix applied.


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.

1 participant