Skip to content

Conversation

@ReoHakase
Copy link
Contributor

@ReoHakase ReoHakase requested a review from dino3616 May 7, 2025 02:35
@ReoHakase ReoHakase self-assigned this May 7, 2025
@vercel
Copy link

vercel bot commented May 7, 2025

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

Name Status Preview Comments Updated (UTC)
lockerai-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 9, 2025 1:40am

@ReoHakase ReoHakase marked this pull request as draft May 7, 2025 02:35
@ReoHakase ReoHakase requested review from Copilot and dino3616 May 9, 2025 00:25
@ReoHakase ReoHakase marked this pull request as ready for review May 9, 2025 00:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces multi-language support into the website and API by adding new GraphQL fields, updating database schemas, and integrating translation logic via langchain. Key changes include:

  • Defining a new JSON scalar type and mapping for multi-language fields (e.g. titleI18n, descriptionI18n).
  • Updating repository, domain, and resolver code to handle multi-language data.
  • Modifying the Prisma schema and migration to add required JSONB columns for i18n support.

Reviewed Changes

Copilot reviewed 84 out of 84 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
apps/website/graphql.schema.json Added the JSON scalar type and multi-language fields for location, name, description, and title.
apps/website/codegen.ts & apps/locker-dashboard/codegen.ts Mapped the new JSON scalar.
apps/api/src/module/lost-item/* Updated lost item use case, repository, domain model, resolvers, and DTOs to support i18n fields.
apps/api/src/module/locker/* Updated locker domain model, repository, and GraphQL object to support i18n fields.
apps/api/src/infra/prisma/schema.prisma & migration.sql Modified the Prisma schema and migration SQL to include new required JSONB columns.
apps/api/src/infra/langchain/langchain.service.ts Added translation functions to generate i18n text.
apps/api/src/common/type/locale.ts, json.scalar.ts, app.module.ts, package.json Integrated new JSON scalar support and i18n types.

Comment on lines +11 to +16
ALTER TABLE "lockers" ADD COLUMN "location_i18n" JSONB NOT NULL,
ADD COLUMN "name_i18n" JSONB NOT NULL;

-- AlterTable
ALTER TABLE "lost_items" ADD COLUMN "description_i18n" JSONB NOT NULL,
ADD COLUMN "title_i18n" JSONB NOT NULL;
Copy link

Copilot AI May 9, 2025

Choose a reason for hiding this comment

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

Adding a new required JSONB column without a default value may cause migration issues if the table contains existing records. Consider specifying a default value (e.g., '{}' or an explicit default translation object) to avoid migration failures.

Suggested change
ALTER TABLE "lockers" ADD COLUMN "location_i18n" JSONB NOT NULL,
ADD COLUMN "name_i18n" JSONB NOT NULL;
-- AlterTable
ALTER TABLE "lost_items" ADD COLUMN "description_i18n" JSONB NOT NULL,
ADD COLUMN "title_i18n" JSONB NOT NULL;
ALTER TABLE "lockers" ADD COLUMN "location_i18n" JSONB NOT NULL DEFAULT '{}',
ADD COLUMN "name_i18n" JSONB NOT NULL DEFAULT '{}';
-- AlterTable
ALTER TABLE "lost_items" ADD COLUMN "description_i18n" JSONB NOT NULL DEFAULT '{}',
ADD COLUMN "title_i18n" JSONB NOT NULL DEFAULT '{}';

Copilot uses AI. Check for mistakes.
Comment on lines +11 to +16
ALTER TABLE "lockers" ADD COLUMN "location_i18n" JSONB NOT NULL,
ADD COLUMN "name_i18n" JSONB NOT NULL;

-- AlterTable
ALTER TABLE "lost_items" ADD COLUMN "description_i18n" JSONB NOT NULL,
ADD COLUMN "title_i18n" JSONB NOT NULL;
Copy link

Copilot AI May 9, 2025

Choose a reason for hiding this comment

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

Adding new required JSONB columns for multi-language fields without default values could lead to migration issues on non-empty tables. It is recommended to provide defaults for both description_i18n and title_i18n to ensure smooth migration.

Suggested change
ALTER TABLE "lockers" ADD COLUMN "location_i18n" JSONB NOT NULL,
ADD COLUMN "name_i18n" JSONB NOT NULL;
-- AlterTable
ALTER TABLE "lost_items" ADD COLUMN "description_i18n" JSONB NOT NULL,
ADD COLUMN "title_i18n" JSONB NOT NULL;
ALTER TABLE "lockers" ADD COLUMN "location_i18n" JSONB NOT NULL DEFAULT '{}',
ADD COLUMN "name_i18n" JSONB NOT NULL DEFAULT '{}';
-- AlterTable
ALTER TABLE "lost_items" ADD COLUMN "description_i18n" JSONB NOT NULL DEFAULT '{}',
ADD COLUMN "title_i18n" JSONB NOT NULL DEFAULT '{}';

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements internationalization support for both the website and API modules, updating code generation, domain models, GraphQL schemas, repositories, and migrations to incorporate i18n text fields.

  • Updated codegen configurations to map JSON scalars.
  • Added i18n fields for lost items and lockers including helper conversions and GraphQL schema changes.
  • Updated Prisma schemas and corresponding migration scripts to support i18n defaults.

Reviewed Changes

Copilot reviewed 90 out of 90 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
apps/website/codegen.ts, apps/locker-dashboard/codegen.ts Added JSON scalar mapping to support i18n values.
apps/api/src/module/lost-item/* Integrated titleI18n and descriptionI18n handling across use cases, repositories, domain models, and GraphQL resolvers.
apps/api/src/module/locker/* Added i18n fields for locker names and locations in models, repositories, and GraphQL objects.
apps/api/src/infra/prisma/schema.prisma and migrations Modified Prisma schema and migrations to include i18n fields with JSON defaults.
apps/api/src/infra/langchain/langchain.service.ts Extended language translation support to generate i18n texts.
Rest Files Updated dependencies and scalar definitions to support GraphQL JSON values.
Comments suppressed due to low confidence (1)

apps/api/src/module/lost-item/repository/impl/lost-item.repository.ts:90

  • [nitpick] The conversion logic applying toI18nText for lost items is repeated across multiple methods. Consider extracting this conversion into a helper function to reduce duplication and improve maintainability.
return lostItems.map((lostItem) => new LostItem(lostItem));

/*
Warnings:

- Added the required column `location_i18n` to the `lockers` table without a default value. This is not possible if the table is not empty.
Copy link

Copilot AI May 9, 2025

Choose a reason for hiding this comment

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

The warnings in this migration comment refer to adding required columns without defaults. Since subsequent migrations set defaults, consider updating or removing these warnings to avoid confusion.

Copilot uses AI. Check for mistakes.
@ReoHakase ReoHakase merged commit 2b756b8 into main May 9, 2025
8 checks passed
@ReoHakase ReoHakase deleted the 24/add-i18n-support branch May 9, 2025 01:45
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.

フロントエンドをi18n対応させる

2 participants