Skip to content
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

feat(mobile): perform actions #3012

Merged
merged 2 commits into from
Mar 10, 2025
Merged

feat(mobile): perform actions #3012

merged 2 commits into from
Mar 10, 2025

Conversation

hyoban
Copy link
Member

@hyoban hyoban commented Mar 10, 2025

Description

PR Type

  • Feature
  • Bugfix
  • Hotfix
  • Other (please describe):

Screenshots (if UI change)

Demo Video (if new feature)

Linked Issues

part of FOL-1492

Additional context

Changelog

  • I have updated the changelog/next.md with my changes.

Copy link

vercel bot commented Mar 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
follow ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 10, 2025 2:44am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
follow-external-ssr ⬜️ Ignored (Inspect) Visit Preview Mar 10, 2025 2:44am

@follow-reviewer-bot
Copy link

Suggested PR Title:

feat(drizzle): add settings to entries and implement migration

Change Summary:
This update introduces the settings field to the entries table in the database schema, allowing for new configurations related to entry summaries and read statuses. A new migration file has been added to support this schema change. Additionally, updates were made to several components and services to handle the new settings properly, improving the overall functionality and user experience of entry management.

Code Review:

Code Review

Issues Requiring Change Requests

  1. File: apps/mobile/drizzle/0016_curious_carnage.sql

    • Line: 1
      • Issue: The SQL file does not end with a newline. This is a common convention that ensures compatibility with various tools and editors.
      • Change Request: Add a newline at the end of the file.
  2. File: apps/mobile/src/modules/entry-content/EntryContentHeaderRightActions.tsx

    • Line: 97
      • Issue: The updated logic no longer awaits the getSummary function, but the function could return a Promise since it interacts with async data.
      • Change Request: If getSummary still needs to return a Promise in the future, ensure the call explicitly handles asynchronous behavior. If the function no longer returns a Promise, update its definition or add comments for clarification.
  3. File: apps/mobile/src/screens/(stack)/entries/[entryId]/index.tsx

    • Line: 22-25 and 28-31
      • Issue: Both useEffect dependencies make asynchronous calls to entrySyncServices.fetchEntrySourceContent and summarySyncService.generateSummary, but there is no error handling or cleanup logic provided.
      • Change Request: Update the useEffect hooks to include proper error handling or cleanup mechanisms to ensure robust behavior during component lifecycle changes.
  4. File: apps/mobile/src/morph/hono.ts

    • Lines: 129, 183
      • Issue: The setting of the settings field to null could cause compatibility issues if other parts of the code expect it to always contain valid JSON or a specific data shape.
      • Change Request: Validate or provide a default value (e.g., an empty object {}) for settings when it is null.
  5. File: apps/mobile/src/database/schemas/index.ts

    • Line: 90
      • Issue: The settings field is being assigned as ActionSettings with JSON parsing but lacks validation, making it potentially unsafe if improperly formatted data is passed.
      • Change Request: Add validation logic to ensure that parsed JSON conforms to the ActionSettings structure.
  6. File: apps/mobile/src/database/schemas/types.ts

    • Line: 34
      • Issue: The HonoApiClient.ActionSettings type is used directly without referencing its potential schema validations.
      • Change Request: Document the expected structure of ActionSettings or validate it against runtime schemas (e.g., using a library like zod or class-validator) to ensure type integrity.

Summary

The points listed above highlight areas where changes are necessary to improve code quality, maintainability, or safety. Please address these issues to ensure the changes in this PR are robust and follow coding conventions.

Let me know if you’d like any further clarification on these points!

Copy link

linear bot commented Mar 10, 2025

FOL-1492 Actions

@hyoban hyoban merged commit 3e313ca into dev Mar 10, 2025
12 checks passed
@hyoban hyoban deleted the feat/mobile-actions branch March 10, 2025 02:52
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