Skip to content

Add fdm-app Add new user management functions and update field handling features#6

Merged
SvenVw merged 81 commits into
mainfrom
initialize-fdm-app
Nov 26, 2024
Merged

Add fdm-app Add new user management functions and update field handling features#6
SvenVw merged 81 commits into
mainfrom
initialize-fdm-app

Conversation

@SvenVw
Copy link
Copy Markdown
Collaborator

@SvenVw SvenVw commented Nov 15, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a comprehensive sidebar component for enhanced navigation.
    • Added a customizable toast notification system for user feedback.
    • Implemented a FieldMap component for visualizing geographical data.
    • Created a Farm component for submitting farm-related information.
    • Introduced user management functionalities with signUpUser and getUserFromSession.
    • Added a Fields component for managing field data in forms.
  • Bug Fixes

    • Enhanced error handling for various components and actions.
  • Documentation

    • Added a README.md file and updated changelogs for better user guidance.
  • Chores

    • Established ESLint and Tailwind CSS configurations for consistent coding standards and styling.

SvenVw and others added 6 commits November 25, 2024 15:31
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Outside diff range and nitpick comments (10)
fdm-app/app/services/fdm.server.tsx (2)

1-1: Remove commented code

Remove the commented import statement as it's not being used.

-// import postgres from 'postgres'

20-30: Use object property shorthand

The connection configuration can be simplified using object property shorthand syntax.

 export const fdm = drizzle({
   connection : {
-    user : user,
-    password : password,
-    host : host,
-    port : port,
-    database : database
+    user,
+    password,
+    host,
+    port,
+    database
   },
   logger: false,
   schema: schema
 })
fdm-app/app/components/blocks/field-map.tsx (2)

12-29: Consider extracting color values to theme constants.

The style definitions are well-structured, but the color values could be extracted to theme constants for better maintainability and reusability across the application.

+const FIELD_COLORS = {
+    fill: "#93c5fd",
+    outline: "#1e3a8a",
+} as const;

 const brpFieldsFillStyle = {
     id: 'brp-fields-fill',
     type: 'fill',
     paint: {
-        'fill-color': "#93c5fd",
+        'fill-color': FIELD_COLORS.fill,
         'fill-opacity': 0.5,
-        'fill-outline-color': "#1e3a8a"
+        'fill-outline-color': FIELD_COLORS.outline
     }
 };

60-68: Consider adding loading state handling for the Source component.

While error handling is implemented, adding a loading state indicator would improve user experience when the GeoJSON data is being loaded.

 <Source
     id="fieldMap"
     type="geojson"
     data={props.b_geojson}
+    loading={<div>Loading map data...</div>}
     onError={(e: Error) => console.error('Source loading error:', e)}>
fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx (5)

1-1: Remove unused import redirect

The redirect import from '@remix-run/node' is not used in this file.

-import { type MetaFunction, type ActionFunctionArgs, type LoaderFunctionArgs, redirect, json } from "@remix-run/node";
+import { type MetaFunction, type ActionFunctionArgs, type LoaderFunctionArgs, json } from "@remix-run/node";

20-25: Enhance SEO meta information

The current title and description are too generic. Consider making them more specific to the farm fields management context.

 export const meta: MetaFunction = () => {
     return [
-        { title: "FDM App" },
-        { name: "description", content: "Welcome to FDM!" },
+        { title: "Manage Farm Fields - FDM App" },
+        { name: "description", content: "Add and manage field information for your farm in the FDM application." },
     ];
 };

38-44: Extract geometry processing to a utility function

The GeoJSON conversion logic should be extracted to a reusable utility function for better maintainability and reusability.

+// Add to a new utility file, e.g., utils/geometry.ts
+export function convertToGeoJSON(geometry: any) {
+    if (!geometry) return null;
+    return wkx.Geometry.parse(geometry).toGeoJSON();
+}

 const fieldsWithGeojson = fields.map(field => {
-    if (!field.b_geometry) {
-        return field;
-    }
-    field.b_geojson = wkx.Geometry.parse(field.b_geometry).toGeoJSON()
+    field.b_geojson = convertToGeoJSON(field.b_geometry);
     return field
 });

46-47: Clarify sorting intention with a named function

The sorting logic would be clearer if extracted to a named function that explains the intent.

-// Sort by created
-fieldsWithGeojson.sort((a, b) => new Date(a.created).getTime() - new Date(b.created).getTime())
+const sortFieldsByCreationDate = (a: any, b: any) => new Date(a.created).getTime() - new Date(b.created).getTime();
+fieldsWithGeojson.sort(sortFieldsByCreationDate);

65-65: Remove commented code

Remove the commented out useNavigation hook if it's not being used.

-    // const navigation = useNavigation();
fdm-app/app/components/blocks/fields.tsx (1)

88-89: Remove or uncomment the CardHeader content.

The commented code in CardHeader should either be removed if not needed or uncommented if it's meant to display the field name and area.

-                        {/* <CardTitle>{props.b_name}</CardTitle>
-                        <CardDescription>{props.b_area} ha</CardDescription> */}
+                        <CardTitle>{props.b_name}</CardTitle>
+                        <CardDescription>{props.b_area} ha</CardDescription>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4367b50 and 3cc1186.

⛔ Files ignored due to path filters (1)
  • fdm-app/app/hooks/use-toast.ts is excluded by !fdm-app/app/hooks/**
📒 Files selected for processing (5)
  • fdm-app/app/components/blocks/field-map.tsx (1 hunks)
  • fdm-app/app/components/blocks/fields.tsx (1 hunks)
  • fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx (1 hunks)
  • fdm-app/app/services/fdm.server.tsx (1 hunks)
  • fdm-app/app/services/session.server.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • fdm-app/app/services/session.server.ts
🧰 Additional context used
📓 Learnings (2)
fdm-app/app/components/blocks/field-map.tsx (1)
Learnt from: SvenVw
PR: SvenVw/fdm#6
File: fdm-app/app/components/blocks/field-map.tsx:0-0
Timestamp: 2024-11-25T14:42:26.660Z
Learning: In `fdm-app/app/components/blocks/field-map.tsx`, explicit cleanup of Mapbox GL resources is not necessary, as `react-map-gl` handles it automatically upon component unmount, and `MapRef` does not have a `remove` method.
fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx (1)
Learnt from: SvenVw
PR: SvenVw/fdm#6
File: fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx:63-99
Timestamp: 2024-11-25T14:50:16.074Z
Learning: i18n will be added in future PRs; for now, hardcoded Dutch text is acceptable until a translation system is implemented.
🪛 Biome (1.9.4)
fdm-app/app/services/fdm.server.tsx

[error] 9-9: Expected an expression but instead found 'throw'.

Expected an expression here.

(parse)


[error] 11-11: Expected an expression but instead found 'throw'.

Expected an expression here.

(parse)


[error] 13-13: Expected an expression but instead found 'throw'.

Expected an expression here.

(parse)


[error] 15-15: Expected an expression but instead found 'throw'.

Expected an expression here.

(parse)


[error] 17-17: Expected an expression but instead found 'throw'.

Expected an expression here.

(parse)


[error] 9-18: This code is unreachable

... because this statement will throw an exception beforehand

(lint/correctness/noUnreachable)


[error] 32-36: This code is unreachable

... because this statement will throw an exception beforehand

(lint/correctness/noUnreachable)

🔇 Additional comments (3)
fdm-app/app/services/fdm.server.tsx (1)

32-36: ⚠️ Potential issue

Wrap async operations in initialization function

Module-level await can be risky and these operations lack proper error handling. Consider:

  1. Moving these operations into an async initialization function
  2. Adding error handling
  3. Making values configurable

The previous review comments about error handling for migrations and fertilizer catalogue are still valid. Additionally, consider this structure:

-// Run migration
-await migrate(fdm, { migrationsFolder: migrationsFolderPath, migrationsSchema: 'fdm-migrations' })
-
-// Add SRM fertilzers to catalogue
-await extendFertilizersCatalogue(fdm, 'srm')
+export async function initializeDatabase() {
+  const catalogueId = process.env.FERTILIZER_CATALOGUE_ID ?? 'srm'
+  const migrationsSchema = process.env.MIGRATIONS_SCHEMA ?? 'fdm-migrations'
+
+  try {
+    // Run migrations if enabled
+    if (process.env.AUTO_MIGRATE === 'true') {
+      await migrate(fdm, { 
+        migrationsFolder: migrationsFolderPath, 
+        migrationsSchema 
+      })
+      console.log('Migrations completed successfully')
+    }
+
+    // Extend fertilizer catalogue
+    await extendFertilizersCatalogue(fdm, catalogueId)
+    console.log(`Successfully extended fertilizer catalogue with ID: ${catalogueId}`)
+  } catch (error) {
+    console.error('Database initialization failed:', error)
+    throw error
+  }
+}
🧰 Tools
🪛 Biome (1.9.4)

[error] 32-36: This code is unreachable

... because this statement will throw an exception beforehand

(lint/correctness/noUnreachable)

fdm-app/app/components/blocks/field-map.tsx (2)

1-10: LGTM! Well-structured imports and type definitions.

The imports are comprehensive and the interface is properly typed with FeatureCollection. Good job renaming Map to MapGL to avoid naming conflicts.


36-43: LGTM! Well-implemented bounds calculation with error handling.

The bounds calculation is properly memoized and includes good error handling with a sensible fallback to world bounds.

Comment thread fdm-app/app/services/fdm.server.tsx Outdated
Comment thread fdm-app/app/components/blocks/field-map.tsx
Comment thread fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx Outdated
Comment thread fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx
Comment thread fdm-app/app/components/blocks/fields.tsx Outdated
Comment thread fdm-app/app/components/blocks/fields.tsx Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
fdm-app/app/components/blocks/fields.tsx (1)

99-100: Remove or uncomment CardHeader content.

The commented code in CardHeader should either be removed if no longer needed or uncommented if it's meant to be used.

-                        {/* <CardTitle>{props.b_name}</CardTitle>
-                        <CardDescription>{props.b_area} ha</CardDescription> */}
+                        <CardTitle>{props.b_name}</CardTitle>
+                        <CardDescription>{props.b_area} ha</CardDescription>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between e742312 and 3649be1.

📒 Files selected for processing (4)
  • fdm-app/app/components/blocks/field-map.tsx (1 hunks)
  • fdm-app/app/components/blocks/fields.tsx (1 hunks)
  • fdm-app/app/root.tsx (1 hunks)
  • fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • fdm-app/app/components/blocks/field-map.tsx
  • fdm-app/app/root.tsx
  • fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx
🔇 Additional comments (2)
fdm-app/app/components/blocks/fields.tsx (2)

59-60: Hard-coded field values are still present.

The issue of hard-coded values for b_area and b_soiltype_agr was previously identified but remains unresolved.


114-125: Hard-coded fruit options in Select component remain unchanged.

The Select component still contains hard-coded fruit options instead of actual crop options.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
fdm-core/CHANGELOG.md (2)

9-9: Add commit hash for consistency

Most changelog entries include commit hashes for reference. Consider adding the commit hash for the b_geometry addition to maintain consistency with other entries.

-Add `b_geometry` for fields
+- <commit-hash>: Add `b_geometry` for fields

10-10: Fix grammatical error

The article "an" should be "a" before "user" as "user" begins with a consonant sound.

-- 7aff5c6: Add `signUpUser` and `getUserFromSession` to create an user and retrieve user information from session
+- 7aff5c6: Add `signUpUser` and `getUserFromSession` to create a user and retrieve user information from session
🧰 Tools
🪛 LanguageTool

[misspelling] ~10-~10: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...serandgetUserFromSession` to create an user and retrieve user information from...

(EN_A_VS_AN)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 3649be1 and 2e66e01.

📒 Files selected for processing (6)
  • fdm-app/CHANGELOG.md (1 hunks)
  • fdm-app/package.json (1 hunks)
  • fdm-core/CHANGELOG.md (1 hunks)
  • fdm-core/package.json (2 hunks)
  • fdm-data/CHANGELOG.md (1 hunks)
  • fdm-data/package.json (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • fdm-app/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • fdm-core/package.json
🧰 Additional context used
📓 Learnings (1)
fdm-app/package.json (1)
Learnt from: SvenVw
PR: SvenVw/fdm#6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
🪛 LanguageTool
fdm-core/CHANGELOG.md

[misspelling] ~10-~10: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...serandgetUserFromSession` to create an user and retrieve user information from...

(EN_A_VS_AN)

fdm-data/CHANGELOG.md

[grammar] ~9-~9: This phrase is duplicated. You should probably use “Updated dependencies” only once.
Context: ...cdc] - Updated dependencies [f9050b0] - Updated dependencies - Updated dependencies [7aff5c6] - @svenvw/fdm-core@0.5.0 #...

(PHRASE_REPETITION)

🔇 Additional comments (8)
fdm-data/package.json (2)

4-4: Verify changelog entries for version 0.3.0

The version bump from 0.2.0 to 0.3.0 follows semantic versioning for new features. Please ensure the changelog is updated to reflect the new functionality.

✅ Verification successful

Version bump is properly documented in the changelog

The changelog contains appropriate entries for version 0.3.0 documenting:

  • Minor changes:
    • License update to MIT
    • Added export for FdmType
  • Updated dependencies including fdm-core
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if CHANGELOG.md exists and contains entries for 0.3.0
fd -t f "CHANGELOG.md" -x grep -A 5 "0.3.0"

Length of output: 257


61-61: Verify fdm-core version and document breaking change

The peer dependency update to require @svenvw/fdm-core@>=0.5.0 is a breaking change that requires users to upgrade. Please ensure:

  1. Version 0.5.0 of fdm-core exists
  2. The breaking change is documented
fdm-app/package.json (6)

1-6: LGTM! Project metadata follows best practices.

The package configuration is well-structured with appropriate scoping, privacy settings, and module type.


15-47: LGTM! Dependencies are well-organized with proper versioning.

Good choice of dependencies:

  • Radix UI components ensure accessibility
  • Specific versions are pinned for stability
  • MapBox integration is properly typed

48-71: LGTM! Development dependencies are comprehensive.

Good setup with:

  • Complete ESLint configuration with necessary plugins
  • TypeScript support
  • Vite and related tools

Note: The invalid version specifier for drizzle-orm has been previously flagged.


76-78: LGTM! Using latest LTS Node.js version.

Good choice using Node.js 20.0.0 or higher, which is the current LTS version.


72-75: Verify workspace dependency versions.

Let's ensure the required minimum versions of workspace dependencies exist and are compatible.

✅ Verification successful

Workspace dependency versions are correctly aligned

The verification confirms that both workspace dependencies exist with the exact minimum versions required:

  • @svenvw/fdm-core is at version 0.5.0
  • @svenvw/fdm-data is at version 0.3.0
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for version declarations in workspace packages
echo "Checking fdm-core version:"
fd -g "package.json" | xargs rg '"version":\s*"0\.5\.' 

echo "Checking fdm-data version:"
fd -g "package.json" | xargs rg '"version":\s*"0\.3\.'

Length of output: 323


7-14: Verify Remix v3 compatibility.

The scripts are well-configured. Given the preparation for Remix v3 migration, let's verify the compatibility of the current setup.

✅ Verification successful

Remix v3 compatibility looks good

The current setup is well-prepared for Remix v3:

  • Using Remix v2.13.1 with Vite configuration in place
  • Vite-based build scripts already configured in package.json
  • Cookie session storage using modern @remix-run/node imports
  • All necessary dependencies for Vite integration present (vite, vite-tsconfig-paths)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if any Remix v3 specific configurations exist
rg -l "remix.*v3|remix.*3\.0" 

# Check for potential breaking changes in Remix v3
fd -e ts -e js -e json | xargs rg "createCookie|createSession|createMemorySession|createFileSession|createCookieSession"

Length of output: 355


Script:

#!/bin/bash
# Check package.json for Remix version and dependencies
cat fdm-app/package.json | jq '.dependencies + .devDependencies'

# Check for Vite configuration which is required for Remix v3
fd vite.config

Length of output: 1946

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