feat(server): placeholder property schema field #1227
Conversation
WalkthroughThe pull request introduces several enhancements across various files, primarily focusing on the integration of new fields and functionalities related to placeholders in property schemas. Key updates include new variables and targets in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
✅ Deploy Preview for reearth-web canceled.
|
Co-authored-by: tomokazu tantaka <t.tantaka@eukarya.io>
hexaforce
force-pushed
the
feat/placeholder-property-schema-field
branch
from
d5a10a1 to
1e440df
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (14)
server/Makefile (2)
14-24: Improve help text formatting and descriptions.While the help text is functional, consider:
- Standardizing the spacing between descriptions
- Adding more detailed descriptions for the new targets:
- What type of schema does
schematypergenerate?- What is the purpose of
PropertySchemaLoader?- @echo " schematyper Generate schema using schematyper" - @echo " dataloaden Generate dataloader for PropertySchemaLoader" + @echo " schematyper Generate Go types from JSON schema for plugin manifests" + @echo " dataloaden Generate GraphQL dataloaders for property schema handling"
53-60: Enhance command robustness and error handling.While the implementation is functional, consider these improvements:
schematyper: - go run $(SCHEMATYPER) -o --package manifest $(MANIFEST_DIR)/schema_translation.go --prefix Translation ./schemas/plugin_manifest_translation.json - go run $(SCHEMATYPER) -o --package manifest $(MANIFEST_DIR)/schema_gen.go ./schemas/plugin_manifest.json + go run $(SCHEMATYPER) -o --package manifest $(MANIFEST_DIR)/schema_translation.go --prefix Translation ./schemas/plugin_manifest_translation.json || exit 1 + go run $(SCHEMATYPER) -o --package manifest $(MANIFEST_DIR)/schema_gen.go ./schemas/plugin_manifest.json || exit 1 dataloaden: - cd internal/adapter/gql/gqldataloader && \ - go run $(DATALOADEN) PropertySchemaLoader ID '$(GQL_PACKAGE)/gqlmodel.PropertySchema' && \ - go run $(DATALOADEN) PropertyLoader ID '$(GQL_PACKAGE)/gqlmodel.Property' + (cd internal/adapter/gql/gqldataloader && \ + go run $(DATALOADEN) PropertySchemaLoader ID '$(GQL_PACKAGE)/gqlmodel.PropertySchema' && \ + go run $(DATALOADEN) PropertyLoader ID '$(GQL_PACKAGE)/gqlmodel.Property') || exit 1Changes:
- Added error handling with
|| exit 1- Grouped the cd command with its operations using parentheses
server/pkg/property/schema_field_builder_test.go (1)
Line range hint
42-57: Add test cases for placeholder functionality.The test suite would benefit from additional test cases that validate the placeholder functionality, particularly:
- A successful case with a simple placeholder
- A case with internationalized placeholder text
- Edge cases (empty placeholder, very long placeholder text)
Here's a suggested test case to add to the tests array:
tests := []struct { // ... existing fields ... }{ + { + Name: "successful with placeholder", + Id: FieldID("test"), + PropertyType: ValueTypeString, + Placeholder: i18n.String{"en": "Enter value", "ja": "値を入力"}, + Expected: struct { + Id: FieldID("test"), + PropertyType: ValueTypeString, + Placeholder: i18n.String{"en": "Enter value", "ja": "値を入力"}, + // ... other expected fields with zero values ... + }, + Err: nil, + }, // ... existing test cases ... }server/e2e/gql_scene_test.go (2)
14-15: Add a descriptive test comment.The test comment should explain the test's purpose and expected behavior.
+// TestGetScenePlaceholderEnglish validates that property schema fields +// have correct English placeholder translations when accessed by an English user. func TestGetScenePlaceholderEnglish(t *testing.T) {
35-52: Refactor field validation for better maintainability.The current implementation using multiple if statements is hard to maintain. Consider using a map for expected values.
+ expectedPlaceholders := map[string]string{ + "tile_type": "please enter tile type", + "tile_url": "please enter tile url", + "tile_zoomLevel": "please enter tile zoomLevel", + "tile_opacity": "please enter tile opacity", + } for _, group := range r.Value("property").Object().Value("schema").Object().Value("groups").Array().Iter() { for _, field := range group.Object().Value("fields").Array().Iter() { fieldId := field.Object().Value("fieldId").Raw().(string) - if fieldId == "tile_type" { - field.Object().Value("translatedPlaceholder").Equal("please enter tile type") - } - if fieldId == "tile_url" { - field.Object().Value("translatedPlaceholder").Equal("please enter tile url") - } - if fieldId == "tile_zoomLevel" { - field.Object().Value("translatedPlaceholder").Equal("please enter tile zoomLevel") - } - if fieldId == "tile_opacity" { - field.Object().Value("translatedPlaceholder").Equal("please enter tile opacity") + if expected, ok := expectedPlaceholders[fieldId]; ok { + field.Object().Value("translatedPlaceholder").Equal(expected) } } }server/schemas/plugin_manifest_translation.json (1)
23-28: Consider adding a description for the placeholder field.To improve schema documentation and help users understand the purpose of this field, consider adding a description in the schema.
"placeholder": { "type": [ "string", "null" - ] + ], + "description": "Provides hint text that appears in the input field when empty" },server/e2e/seeder.go (1)
Line range hint
71-134: LGTM: Consider adding documentation for the setup sequenceThe extracted
baseSetupfunction is well-structured and maintains the original functionality. Consider adding a doc comment to describe the setup sequence and dependencies between the created entities.Add a documentation comment like this:
+// baseSetup creates a complete test environment with interconnected entities: +// 1. Creates a workspace with the provided user as owner +// 2. Creates a project in the workspace +// 3. Creates a dataset schema with test fields +// 4. Creates a dataset using the schema func baseSetup(ctx context.Context, r *repo.Container, u *user.User) error {server/gql/property.graphql (1)
42-42: Consider making theplaceholderfield nullable.While the placeholder support is well-implemented following the existing translation pattern, consider making the base
placeholderfield nullable (Stringinstead ofString!). This would provide more flexibility, as not all fields may require placeholders.- placeholder: String! + placeholder: StringAlso applies to: 47-47, 50-50
server/schemas/plugin_manifest.json (1)
117-122: Consider adding field-specific placeholder constraints.Given that placeholders are intended for specific fields (tile_type, tile_url, tile_zoomLevel, tile_opacity), consider adding field-specific pattern constraints or enums to ensure placeholder text follows expected formats. For example:
- tile_url placeholders might benefit from a URL format pattern
- tile_zoomLevel placeholders might benefit from a numeric format pattern
Example enhancement:
"placeholder": { "type": [ "string", "null" - ] + ], + "pattern": "^[\\w\\s-:./]+$" }server/pkg/builtin/manifest_ja.yml (1)
175-175: LGTM: Placeholders for Cesium extension are well-defined and terminology is improvedThe added placeholders provide clear guidance, and the terminology change from "不透明性" to "不透明度" for opacity improves consistency across the interface.
Consider using the same pattern for min/max level placeholders as used in the Cesium Beta extension for consistency:
- placeholder: 最小レベルを入力 + placeholder: ズームレベルを入力 - placeholder: 最大レベルを入力 + placeholder: ズームレベルを入力Also applies to: 188-188, 191-191, 194-194, 197-198
server/pkg/builtin/manifest.yml (2)
50-50: LGTM! Consider improving placeholder text format.The placeholders are well-placed and provide clear guidance. However, consider capitalizing the first letter and removing "please" for a more concise and professional format:
- placeholder: please enter tile type + placeholder: Enter tile type - placeholder: please enter tile url + placeholder: Enter tile URL - placeholder: please enter tile zoomLevel + placeholder: Enter tile zoom level - placeholder: please enter tile opacity + placeholder: Enter tile opacityAlso applies to: 75-75, 84-84, 93-93
336-336: Maintain consistent placeholder format across all fields.The placeholders are consistently applied across both tile groups. Apply the same formatting improvements here:
- placeholder: please enter tile type + placeholder: Enter tile type - placeholder: please enter tile url + placeholder: Enter tile URL - placeholder: please enter tile minLevel + placeholder: Enter minimum level - placeholder: please enter tile maxLevel + placeholder: Enter maximum level - placeholder: please enter tile opacity + placeholder: Enter tile opacityAlso applies to: 361-361, 369-369, 375-375, 381-381
server/pkg/plugin/manifest/schema_translation_i18n.go (2)
192-235: Refactor to reduce code duplication insetPropertySchemamethodThe
setPropertySchemamethod contains repeated code blocks for handling fields likeTitle,Description,Prefix,Suffix, andPlaceholder. Refactoring can improve maintainability and readability.Consider creating a helper function to assign translations:
func assignI18nString(dst *i18n.String, src *string, lang string) { if src != nil { if *dst == nil { *dst = i18n.String{} } (*dst)[lang] = *src } }Refactor the method using this helper:
assignI18nString(&tg.Title, g.Title, l) assignI18nString(&tg.Description, g.Description, l) // ... assignI18nString(&tf.Title, f.Title, l) assignI18nString(&tf.Description, f.Description, l) assignI18nString(&tf.Prefix, f.Prefix, l) assignI18nString(&tf.Suffix, f.Suffix, l) assignI18nString(&tf.Placeholder, f.Placeholder, l)This approach reduces duplication and enhances code clarity.
40-74: Add unit tests for theTranslatedmethodThe
Translatedmethod is central to constructing theTranslatedRoot. Adding unit tests will ensure its correctness across different scenarios, such as empty translations, multiple languages, and nested extensions.Consider implementing unit tests that cover:
- Empty
TranslationMapinputs.- Single and multiple language entries.
- Presence of extensions and property schemas.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (23)
server/Makefile(2 hunks)server/e2e/gql_project_import_test.go(4 hunks)server/e2e/gql_property_test.go(1 hunks)server/e2e/gql_scene_test.go(1 hunks)server/e2e/seeder.go(2 hunks)server/gql/property.graphql(3 hunks)server/gqlgen.yml(1 hunks)server/internal/adapter/gql/generated.go(28 hunks)server/internal/adapter/gql/gqlmodel/convert_property.go(2 hunks)server/internal/adapter/gql/gqlmodel/models_gen.go(1 hunks)server/internal/adapter/gql/resolver_property_schema.go(1 hunks)server/internal/infrastructure/mongo/mongodoc/property_schema.go(3 hunks)server/pkg/builtin/manifest.yml(7 hunks)server/pkg/builtin/manifest_ja.yml(4 hunks)server/pkg/plugin/manifest/convert.go(2 hunks)server/pkg/plugin/manifest/schema_gen.go(2 hunks)server/pkg/plugin/manifest/schema_translation.go(2 hunks)server/pkg/plugin/manifest/schema_translation_i18n.go(1 hunks)server/pkg/property/schema_field.go(2 hunks)server/pkg/property/schema_field_builder.go(1 hunks)server/pkg/property/schema_field_builder_test.go(4 hunks)server/schemas/plugin_manifest.json(1 hunks)server/schemas/plugin_manifest_translation.json(2 hunks)
🔇 Additional comments (40)
server/internal/adapter/gql/generated.go (5)
9752-9760: LGTM! Schema definition changes look good.
The new placeholder fields are properly defined with correct nullability constraints:
placeholder: String!- Required fieldallTranslatedPlaceholder: TranslatedString- Optional translationstranslatedPlaceholder(lang: Lang): String!- Required translation for given language
43649-43691: LGTM! Proper null handling implementation.
The placeholder field resolver correctly implements:
- Null checking with appropriate error messages
- Type assertions
- Error recovery through defer/recover pattern
44028-44081: LGTM! Robust error handling in translatedPlaceholder resolver.
The implementation includes:
- Proper middleware integration
- Language tag handling
- Comprehensive error handling with recovery
- Correct field context setup
72837-72872: LGTM! Well-implemented concurrency support.
The concurrent field resolution implementation:
- Properly handles deferred fields
- Includes atomic counter for invalid fields
- Implements proper error recovery
- Maintains consistency with other translated fields
1691-1691: Verify resolver implementation.
The interface defines TranslatedPlaceholder, but let's verify the actual resolver implementation exists.
✅ Verification successful
The resolver implementation exists and is correctly implemented
The implementation is found in server/internal/adapter/gql/resolver_property_schema.go:
func (r *propertySchemaFieldResolver) TranslatedPlaceholder(ctx context.Context, obj *gqlmodel.PropertySchemaField, lang *language.Tag) (string, error) {
if s, ok := obj.AllTranslatedPlaceholder[getLang(ctx, lang)]; ok {
return s, nil
}
return obj.Placeholder, nil
}The implementation follows the expected interface signature and provides proper fallback behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the TranslatedPlaceholder resolver implementation
ast-grep --pattern 'func (*_) TranslatedPlaceholder(ctx context.Context, obj *gqlmodel.PropertySchemaField, lang *language.Tag) (string, error)'
Length of output: 146
Script:
#!/bin/bash
# Let's try a broader search for TranslatedPlaceholder implementations
ast-grep --pattern 'func $_($$$) TranslatedPlaceholder($$$) (string, error)'
# Also search for the interface definition to understand the contract
ast-grep --pattern 'type PropertySchemaFieldResolver interface {
$$$
TranslatedPlaceholder($$$)
$$$
}'
# Backup search using ripgrep
rg "TranslatedPlaceholder" -A 5 -B 5
Length of output: 23018
server/e2e/gql_property_test.go (1)
30-30: LGTM: Placeholder field added successfully.
The addition of the placeholder field to the GraphQL query aligns with the PR objectives.
server/Makefile (3)
3-7: LGTM! Well-structured variable declarations.
The variables are properly defined using immediate evaluation (:=) and follow consistent naming conventions.
62-62: LGTM! Proper .PHONY declaration.
All targets, including the new ones, are correctly declared as phony targets.
53-60: Verify integration with CI/CD workflows.
Please ensure that these new targets are properly integrated into your CI/CD workflows if they need to be run during the build process.
server/pkg/property/schema_field_builder_test.go (1)
18-18: LGTM! The placeholder field integration is well-structured.
The changes consistently integrate the new placeholder field across the test structure, builder chain, and assertions, maintaining the existing patterns and type safety.
Also applies to: 34-34, 76-76, 90-90
server/e2e/gql_scene_test.go (1)
1-13: LGTM! Well-structured imports and helpful database setup comment.
The package declaration and imports are clean and appropriate for the test file's purpose.
server/schemas/plugin_manifest_translation.json (1)
23-28: LGTM! The placeholder property is correctly defined.
The addition follows the schema's established patterns and maintains backward compatibility by allowing null values.
server/pkg/property/schema_field_builder.go (1)
131-134: Consider handling nil input for consistency.
The implementation looks good and follows the builder pattern consistently. However, consider handling nil input similar to other methods in this builder for better defensive programming.
Here's a suggested improvement:
func (b *SchemaFieldBuilder) Placeholder(placeholder i18n.String) *SchemaFieldBuilder {
- b.p.placeholder = placeholder.Clone()
+ if placeholder == nil {
+ b.p.placeholder = nil
+ } else {
+ b.p.placeholder = placeholder.Clone()
+ }
return b
}Let's verify if this method is properly tested:
server/e2e/seeder.go (3)
17-17: LGTM: Appropriate import for language support
The addition of the golang.org/x/text/language package is correct for implementing language tag support.
52-53: LGTM: Clean refactoring of setup logic
The extraction of setup logic into a separate function reduces code duplication while maintaining the original functionality.
55-69: Consider adding test cases for language-specific seeding
The new baseSeederWithLang function looks well-implemented. However, since this is test infrastructure code, it's important to verify that it correctly sets up test data with different languages.
Let's check if there are corresponding test cases:
Would you like me to help create test cases that verify the seeding works correctly with different languages?
✅ Verification successful
Let me check how baseSeederWithLang is used in the test file to verify if it's adequately tested.
Test coverage for language-specific seeding is already in place
The function is well-tested with both English and Japanese language configurations in gql_scene_test.go. The tests verify the seeding functionality by:
- Testing with English locale using
language.English - Testing with Japanese locale using
language.Japanese - Validating scene retrieval with corresponding language settings
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for language-specific test cases
# Test: Search for test files that use baseSeederWithLang
rg -l "baseSeederWithLang" --type go
Length of output: 86
Script:
#!/bin/bash
# Check the usage context of baseSeederWithLang in the test file
rg "baseSeederWithLang" server/e2e/gql_scene_test.go -B 2 -A 5
Length of output: 622
server/pkg/property/schema_field.go (3)
12-12: LGTM: Well-structured field addition
The placeholder field is appropriately typed as i18n.String for internationalization support and follows the established pattern of the struct's field organization.
109-114: LGTM: Method implementation follows established patterns
The Placeholder() method is well-implemented with proper nil checks and consistent with other getter methods in the struct, including the use of Clone() for safe value return.
109-114: Verify companion method requirements
While the getter implementation is solid, consider if a corresponding SetPlaceholder method might be needed, similar to how SetTitle and SetDescription exist for other i18n.String fields.
server/pkg/plugin/manifest/schema_gen.go (2)
68-68: LGTM! The new Placeholder field is properly defined
The addition of the Placeholder field with type *string and JSON tag json:"placeholder,omitempty" is consistent with:
- The PR objectives to support placeholder text
- The pattern used for other optional string fields in the schema
- Best practices for optional fields by using the
omitemptytag
3-3: Note: This is a generated file
This file is automatically generated by the schematyper tool. Any changes should be made to the source schema file (./schemas/plugin_manifest.json) rather than directly to this file.
✅ Verification successful
Let me verify the schema file structure more thoroughly to understand if the field exists under a different path or if there's an issue with the generated code.
Let me verify the generated code to compare it with the schema definition.
Generated code matches the schema definition
The placeholder field is correctly defined in both the source schema (plugin_manifest.json) under definitions.propertySchemaField.properties.placeholder and the generated code as an optional string field (*string) with proper JSON tags. The generation is working as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the source schema file exists and contains the placeholder field definition
fd -e json 'plugin_manifest.json' -x cat {} | jq '.definitions.PropertySchemaField.properties.placeholder'
Length of output: 111
Script:
#!/bin/bash
# First, let's see the full schema file to understand its structure
fd -e json 'plugin_manifest.json' -x cat {}
Length of output: 9446
Script:
#!/bin/bash
# Check the generated struct definition for PropertySchemaField
rg -A 20 "type PropertySchemaField struct" server/pkg/plugin/manifest/schema_gen.go
Length of output: 1188
server/gql/property.graphql (1)
303-305: LGTM! Improved mutation signature formatting.
The reformatting of the linkDatasetToPropertyValue mutation signature enhances readability while maintaining functionality.
server/internal/infrastructure/mongo/mongodoc/property_schema.go (4)
32-32: LGTM: Well-structured placeholder field addition
The new Placeholder field is correctly implemented as a string map to support internationalization, consistent with other translatable fields in the struct.
81-82: LGTM: Clean constructor implementation
The constructor properly initializes the new Placeholder field while cleaning up the Description assignment.
174-174: LGTM: Proper model conversion implementation
The Placeholder field is correctly integrated into the model conversion chain.
32-32: Verify test coverage for placeholder functionality
While the implementation looks correct, let's ensure proper test coverage exists for the new placeholder functionality.
Also applies to: 81-82, 174-174
✅ Verification successful
Placeholder functionality is well-tested across multiple layers
The codebase shows comprehensive test coverage for the placeholder functionality:
- Unit tests in
schema_field_builder_test.goverify placeholder field construction and validation - End-to-end tests in
gql_scene_test.gocover:- Placeholder translations in English
- Placeholder translations in Japanese
- Multiple field types (tile_type, tile_url, tile_zoomLevel, tile_opacity)
- GraphQL schema tests in
gql_project_import_test.goverify placeholder field presence
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for tests related to the placeholder field
echo "Searching for placeholder-related tests..."
rg -l "Placeholder" --type go --glob "*_test.go"
# Look for specific test cases
echo -e "\nSearching for specific placeholder test assertions..."
rg "Placeholder|placeholder" --type go --glob "*_test.go" -C 2
Length of output: 5058
server/gqlgen.yml (1)
217-218: LGTM! Addition follows the established translation pattern.
The new translatedPlaceholder field follows the same pattern as other translated fields in the PropertySchemaField model.
Let's verify the resolver implementation:
✅ Verification successful
Resolver implementation is complete and follows the established pattern ✅
The verification confirms:
- The
TranslatedPlaceholdermethod is properly defined in thePropertySchemaFieldResolverinterface - The implementation in
propertySchemaFieldResolverfollows the same pattern as other translated fields:- Checks for translated value in
AllTranslatedPlaceholdermap using the provided language - Falls back to the default
Placeholdervalue if no translation exists
- Checks for translated value in
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the resolver for translatedPlaceholder is implemented
# Test 1: Verify the resolver interface includes TranslatedPlaceholder method
echo "Checking resolver interface..."
ast-grep --pattern 'type PropertySchemaFieldResolver interface {
$$$
TranslatedPlaceholder($$$) $$$
$$$
}'
# Test 2: Verify the actual implementation exists
echo "Checking implementation..."
ast-grep --pattern 'func ($$$) TranslatedPlaceholder($$$) $$$'
Length of output: 1555
server/schemas/plugin_manifest.json (2)
117-122: LGTM! Well-structured placeholder property definition.
The new placeholder property is correctly defined with appropriate type constraints allowing both string and null values. The placement is logical, grouped with other text-related fields.
117-122: Verify placeholder usage in manifest files.
The placeholder integration looks good. Let's verify its usage in the manifest files mentioned in the PR objectives.
✅ Verification successful
Let me gather more information to verify the placeholder usage in the English manifest file as well.
Let me try a different approach to check the manifest files for tile-related fields.
Placeholder integration is properly implemented in manifest files
The verification shows that placeholder fields are correctly used in both manifest files:
-
English manifest (manifest.yml) has placeholders for tile-related fields:
tile_type: "please enter tile type"tile_url: "please enter tile url"tile_zoomLevel: "please enter tile zoomLevel"tile_opacity: "please enter tile opacity"
-
Japanese manifest (manifest_ja.yml) has corresponding placeholders:
tile_url: "タイルのURLを入力"tile_opacity: "不透明度を入力"
The placeholder integration follows the schema definition and maintains consistency across language versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the usage of placeholders in manifest files for tile-related fields
# Test: Search for placeholder definitions in manifest files
echo "Checking manifest files for placeholder usage..."
rg -A 1 "tile_(type|url|zoomLevel|opacity).*:" server/pkg/builtin/manifest*.yml
Length of output: 1072
Script:
#!/bin/bash
# Check the English manifest file for tile-related fields and their placeholders
rg -A 2 "tile_(type|url|zoomLevel|opacity):" server/pkg/builtin/manifest.yml
Length of output: 78
Script:
#!/bin/bash
# Check both manifest files for any tile-related fields using a more general pattern
echo "Checking manifest files for tile fields..."
rg -l "tile" server/pkg/builtin/manifest*.yml
echo -e "\nChecking content of manifest.yml..."
cat server/pkg/builtin/manifest.yml
echo -e "\nChecking content of manifest_ja.yml..."
cat server/pkg/builtin/manifest_ja.yml
Length of output: 122300
server/e2e/gql_project_import_test.go (3)
11-11: LGTM: Appropriate language handling implementation.
The addition of the language package and its usage for specifying English as the default language is well-implemented and aligns with the localization requirements.
Also applies to: 40-40
87-94: Consider adding test coverage for non-English languages.
While the function modification correctly implements language support, consider adding test cases for non-English languages (e.g., Japanese) to ensure proper localization handling across different languages.
Would you like me to help create additional test cases for Japanese language support? This would align with the Japanese translations mentioned in the PR objectives.
262-263: LGTM: Well-structured GraphQL query updates.
The addition of placeholder and translatedPlaceholder fields to the PropertySchemaFieldFragment is well-implemented and properly integrates with the language parameter.
server/pkg/plugin/manifest/convert.go (1)
347-355: LGTM: Clean implementation of i18n placeholder support.
The implementation follows the established pattern for internationalized strings and properly handles both translated and default values.
server/internal/adapter/gql/gqlmodel/convert_property.go (1)
277-277: LGTM! Implementation follows established patterns
The addition of Placeholder and AllTranslatedPlaceholder fields is implemented correctly, following the same pattern used for other translated fields in the schema. The implementation properly handles both the default string value and the full translation object.
Also applies to: 295-295
server/pkg/builtin/manifest_ja.yml (1)
26-26: LGTM: Placeholders for Cesium Beta extension are well-defined
The added placeholders provide clear guidance in Japanese for each field:
- タイルのタイプを選択 (Select tile type)
- タイルのURLを入力 (Enter tile URL)
- ズームレベルを入力 (Enter zoom level)
- 不透明度を入力 (Enter opacity)
Also applies to: 38-38, 41-41, 45-45
server/pkg/builtin/manifest.yml (1)
2578-2578: Verify the intention behind empty choices array.
The showLayers field's choices array is set to empty. Please confirm if this is intentional and whether the options will be populated dynamically at runtime.
Run this script to check for any references to this field's choices:
server/internal/adapter/gql/gqlmodel/models_gen.go (1)
1310-1318: Well-structured implementation of placeholder support!
The implementation follows good practices:
- Consistent naming conventions with other translation-related fields
- Appropriate type choices for each field
- Logical grouping of related fields
- Complete internationalization support through
AllTranslatedPlaceholderandTranslatedPlaceholderfields
server/pkg/plugin/manifest/schema_translation.go (2)
17-17: Addition of Placeholder field looks good
The new Placeholder field in TranslationPropertySchemaField is correctly added and will enhance localization capabilities.
20-20: Repositioning of Title field is acceptable
Reordering the Title field in the struct does not impact functionality.
server/pkg/plugin/manifest/schema_translation_i18n.go (1)
186-188:
Fix potential nil pointer dereference in setPropertySchema method
In the setPropertySchema method, the condition if t == nil || *t == nil can cause a nil pointer dereference when t is nil. Attempting to dereference *t when t is nil will result in a panic.
Modify the condition to handle the nil case safely:
for l, tl := range schemas {
for gid, g := range tl {
+ if t == nil {
+ continue
+ }
if *t == nil {
*t = TranslatedPropertySchema{}
}
// Rest of the code...Ensure that t is not nil before dereferencing it to maintain robustness.
Likely invalid or redundant comment.
| @@ -27,6 +27,7 @@ func updatePropertyValue(e *httpexpect.Expect, propertyID, schemaGroupID, itemID | |||
| ui | |||
| min | |||
| max | |||
| placeholder | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding translatedPlaceholder field for complete test coverage.
The AI summary indicates that both placeholder and translatedPlaceholder fields were added to support localization. To ensure comprehensive test coverage, consider adding the translatedPlaceholder field to the GraphQL query.
fields{
fieldId
type
title
description
prefix
suffix
defaultValue
ui
min
max
placeholder
+ translatedPlaceholder
}Committable suggestion skipped: line range outside the PR's diff.
| pID := createProjectWithExternalImage(e, "test") | ||
| _, _, sID := createScene(e, pID) | ||
| r := getScene(e, sID, language.English.String()) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for project and scene creation.
The test should verify that project and scene creation operations succeed.
-pID := createProjectWithExternalImage(e, "test")
-_, _, sID := createScene(e, pID)
-r := getScene(e, sID, language.English.String())
+pID := createProjectWithExternalImage(e, "test")
+if pID == "" {
+ t.Fatal("failed to create project")
+}
+_, _, sID := createScene(e, pID)
+if sID == "" {
+ t.Fatal("failed to create scene")
+}
+r := getScene(e, sID, language.English.String())
+if r == nil {
+ t.Fatal("failed to get scene")
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pID := createProjectWithExternalImage(e, "test") | |
| _, _, sID := createScene(e, pID) | |
| r := getScene(e, sID, language.English.String()) | |
| pID := createProjectWithExternalImage(e, "test") | |
| if pID == "" { | |
| t.Fatal("failed to create project") | |
| } | |
| _, _, sID := createScene(e, pID) | |
| if sID == "" { | |
| t.Fatal("failed to create scene") | |
| } | |
| r := getScene(e, sID, language.English.String()) | |
| if r == nil { | |
| t.Fatal("failed to get scene") | |
| } |
| package e2e | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/reearth/reearth/server/internal/app/config" | ||
| "github.com/reearth/reearth/server/internal/usecase/repo" | ||
| "golang.org/x/text/language" | ||
| ) | ||
|
|
||
| // export REEARTH_DB=mongodb://localhost | ||
|
|
||
| // go test -v -run TestGetScenePlaceholderEnglish ./e2e/... | ||
| func TestGetScenePlaceholderEnglish(t *testing.T) { | ||
|
|
||
| e := StartServer( | ||
| t, | ||
| &config.Config{ | ||
| Origins: []string{"https://example.com"}, | ||
| AuthSrv: config.AuthSrvConfig{ | ||
| Disabled: true, | ||
| }, | ||
| }, | ||
| true, | ||
| // English user Seeder | ||
| func(ctx context.Context, r *repo.Container) error { | ||
| return baseSeederWithLang(ctx, r, language.English) | ||
| }, | ||
| ) | ||
| pID := createProjectWithExternalImage(e, "test") | ||
| _, _, sID := createScene(e, pID) | ||
| r := getScene(e, sID, language.English.String()) | ||
|
|
||
| for _, group := range r.Value("property").Object().Value("schema").Object().Value("groups").Array().Iter() { | ||
| for _, field := range group.Object().Value("fields").Array().Iter() { | ||
| fieldId := field.Object().Value("fieldId").Raw().(string) | ||
|
|
||
| if fieldId == "tile_type" { | ||
| field.Object().Value("translatedPlaceholder").Equal("please enter tile type") | ||
| } | ||
| if fieldId == "tile_url" { | ||
| field.Object().Value("translatedPlaceholder").Equal("please enter tile url") | ||
| } | ||
| if fieldId == "tile_zoomLevel" { | ||
| field.Object().Value("translatedPlaceholder").Equal("please enter tile zoomLevel") | ||
| } | ||
| if fieldId == "tile_opacity" { | ||
| field.Object().Value("translatedPlaceholder").Equal("please enter tile opacity") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| // go test -v -run TestGetScenePlaceholderJapanese ./e2e/... | ||
|
|
||
| func TestGetScenePlaceholderJapanese(t *testing.T) { | ||
|
|
||
| e := StartServer( | ||
| t, | ||
| &config.Config{ | ||
| Origins: []string{"https://example.com"}, | ||
| AuthSrv: config.AuthSrvConfig{ | ||
| Disabled: true, | ||
| }, | ||
| }, | ||
| true, | ||
| // Japanese user Seeder | ||
| func(ctx context.Context, r *repo.Container) error { | ||
| return baseSeederWithLang(ctx, r, language.Japanese) | ||
| }, | ||
| ) | ||
| pID := createProjectWithExternalImage(e, "test") | ||
| _, _, sID := createScene(e, pID) | ||
| r := getScene(e, sID, language.Japanese.String()) | ||
|
|
||
| for _, group := range r.Value("property").Object().Value("schema").Object().Value("groups").Array().Iter() { | ||
| for _, field := range group.Object().Value("fields").Array().Iter() { | ||
| fieldId := field.Object().Value("fieldId").Raw().(string) | ||
|
|
||
| if fieldId == "tile_type" { | ||
| field.Object().Value("translatedPlaceholder").Equal("タイルのタイプを選択") | ||
| } | ||
| if fieldId == "tile_url" { | ||
| field.Object().Value("translatedPlaceholder").Equal("タイルのURLを入力") | ||
| } | ||
| if fieldId == "tile_zoomLevel" { | ||
| field.Object().Value("translatedPlaceholder").Equal("ズームレベルを入力") | ||
| } | ||
| if fieldId == "tile_opacity" { | ||
| field.Object().Value("translatedPlaceholder").Equal("不透明度を入力") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Tests need to cover additional placeholder fields
The current tests only verify 4 placeholder fields (tile_type, tile_url, tile_zoomLevel, tile_opacity), but the manifest files show additional fields that need testing:
tile_minLevel("please enter tile minLevel" / "最小レベルを入力")tile_maxLevel("please enter tile maxLevel" / "最大レベルを入力")
The tests should be extended to cover all placeholder fields defined in the manifest files to ensure complete translation coverage.
🔗 Analysis chain
Verify test coverage for all placeholder fields.
The tests appropriately validate the placeholder translations for tile-related fields. However, let's ensure we're testing all fields mentioned in the manifest files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all fields with placeholders in manifest files are covered by tests
# Test: Search for placeholder definitions in manifest files
echo "Searching for placeholder definitions in manifest files..."
rg -A 1 "placeholder:" "server/pkg/builtin/manifest*.yml"
Length of output: 257
Script:
#!/bin/bash
# Let's first find the manifest files in the repository
fd -e yml -e yaml manifest
# Then search for placeholder definitions in all yaml/yml files
echo "Searching for placeholder definitions in all yaml/yml files..."
rg -A 1 "placeholder:" -g "*.{yml,yaml}"
# Also search for placeholder-related code in Go files
echo "Searching for placeholder-related code in Go files..."
rg "placeholder" -t go -A 2
Length of output: 9190
| func TestGetScenePlaceholderJapanese(t *testing.T) { | ||
|
|
||
| e := StartServer( | ||
| t, | ||
| &config.Config{ | ||
| Origins: []string{"https://example.com"}, | ||
| AuthSrv: config.AuthSrvConfig{ | ||
| Disabled: true, | ||
| }, | ||
| }, | ||
| true, | ||
| // Japanese user Seeder | ||
| func(ctx context.Context, r *repo.Container) error { | ||
| return baseSeederWithLang(ctx, r, language.Japanese) | ||
| }, | ||
| ) | ||
| pID := createProjectWithExternalImage(e, "test") | ||
| _, _, sID := createScene(e, pID) | ||
| r := getScene(e, sID, language.Japanese.String()) | ||
|
|
||
| for _, group := range r.Value("property").Object().Value("schema").Object().Value("groups").Array().Iter() { | ||
| for _, field := range group.Object().Value("fields").Array().Iter() { | ||
| fieldId := field.Object().Value("fieldId").Raw().(string) | ||
|
|
||
| if fieldId == "tile_type" { | ||
| field.Object().Value("translatedPlaceholder").Equal("タイルのタイプを選択") | ||
| } | ||
| if fieldId == "tile_url" { | ||
| field.Object().Value("translatedPlaceholder").Equal("タイルのURLを入力") | ||
| } | ||
| if fieldId == "tile_zoomLevel" { | ||
| field.Object().Value("translatedPlaceholder").Equal("ズームレベルを入力") | ||
| } | ||
| if fieldId == "tile_opacity" { | ||
| field.Object().Value("translatedPlaceholder").Equal("不透明度を入力") | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication between test functions.
Both test functions share similar structure and logic. Consider refactoring to use a test helper function.
+type placeholderTest struct {
+ name string
+ lang language.Tag
+ expected map[string]string
+}
+
+func testScenePlaceholder(t *testing.T, tt placeholderTest) {
+ e := StartServer(
+ t,
+ &config.Config{
+ Origins: []string{"https://example.com"},
+ AuthSrv: config.AuthSrvConfig{
+ Disabled: true,
+ },
+ },
+ true,
+ func(ctx context.Context, r *repo.Container) error {
+ return baseSeederWithLang(ctx, r, tt.lang)
+ },
+ )
+
+ pID := createProjectWithExternalImage(e, "test")
+ if pID == "" {
+ t.Fatal("failed to create project")
+ }
+
+ _, _, sID := createScene(e, pID)
+ if sID == "" {
+ t.Fatal("failed to create scene")
+ }
+
+ r := getScene(e, sID, tt.lang.String())
+ if r == nil {
+ t.Fatal("failed to get scene")
+ }
+
+ for _, group := range r.Value("property").Object().Value("schema").Object().Value("groups").Array().Iter() {
+ for _, field := range group.Object().Value("fields").Array().Iter() {
+ fieldId := field.Object().Value("fieldId").Raw().(string)
+ if expected, ok := tt.expected[fieldId]; ok {
+ field.Object().Value("translatedPlaceholder").Equal(expected)
+ }
+ }
+ }
+}
+
+func TestGetScenePlaceholder(t *testing.T) {
+ tests := []placeholderTest{
+ {
+ name: "English",
+ lang: language.English,
+ expected: map[string]string{
+ "tile_type": "please enter tile type",
+ "tile_url": "please enter tile url",
+ "tile_zoomLevel": "please enter tile zoomLevel",
+ "tile_opacity": "please enter tile opacity",
+ },
+ },
+ {
+ name: "Japanese",
+ lang: language.Japanese,
+ expected: map[string]string{
+ "tile_type": "タイルのタイプを選択",
+ "tile_url": "タイルのURLを入力",
+ "tile_zoomLevel": "ズームレベルを入力",
+ "tile_opacity": "不透明度を入力",
+ },
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ testScenePlaceholder(t, tt)
+ })
+ }
+}Committable suggestion skipped: line range outside the PR's diff.
| func (r *propertySchemaFieldResolver) TranslatedPlaceholder(ctx context.Context, obj *gqlmodel.PropertySchemaField, lang *language.Tag) (string, error) { | ||
| if s, ok := obj.AllTranslatedPlaceholder[getLang(ctx, lang)]; ok { | ||
| return s, nil | ||
| } | ||
| return obj.Placeholder, nil | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
TranslatedPlaceholder implementation is correct but lacks test coverage
The implementation follows the same pattern as TranslatedTitle and TranslatedDescription methods in the same file, which is correct. However:
- No translation-related tests exist in
resolver_property_test.go - The new
TranslatedPlaceholdermethod lacks test coverage while being part of the public API
Suggested actions:
- Add unit tests for the
TranslatedPlaceholdermethod inresolver_property_test.go, following the same pattern as other translation tests in the codebase - Test both successful translation and fallback scenarios
🔗 Analysis chain
Implementation looks good and follows established patterns!
The TranslatedPlaceholder method correctly implements the translation logic, following the same pattern as other translation methods in the file. It properly handles:
- Language fallback using the existing
getLanghelper - Default placeholder when no translation exists
Let's verify the test coverage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing tests of the TranslatedPlaceholder functionality
rg -A 5 "TestTranslatedPlaceholder|test.*Placeholder.*Translation" --type go
Length of output: 78
Script:
#!/bin/bash
# Let's first check if there are any test files related to property schema resolvers
fd "resolver.*test.go" --type f
# Then let's look for any existing translation-related tests to understand the testing pattern
rg -l "Test.*Translated" --type go
# Finally, let's check the actual implementation of other translation methods
ast-grep --pattern 'func \(r \*propertySchemaFieldResolver\) Translated.* \(ctx context.Context, obj \*gqlmodel.PropertySchemaField, lang \*language.Tag\) \(string, error\)'
Length of output: 385
Script:
#!/bin/bash
# Let's check the content of resolver_property_test.go to see if it contains any translation-related tests
rg -A 10 "func Test.*Translation|func Test.*Translated" server/internal/adapter/gql/resolver_property_test.go
# Let's also check for any existing test setup or test data for translations
rg "AllTranslated.*Placeholder|Translation.*Test" server/internal/adapter/gql/resolver_property_test.go
# Let's verify if there are similar translation methods in the same file
rg -A 5 "func.*Translated.*\(ctx context\.Context, obj \*gqlmodel\.PropertySchemaField" server/internal/adapter/gql/resolver_property_schema.go
Length of output: 1436
| @@ -39,12 +39,15 @@ type PropertySchemaField { | |||
| ui: PropertySchemaFieldUI | |||
| min: Float | |||
| max: Float | |||
| placeholder: String! | |||
There was a problem hiding this comment.
💡 Codebase verification
Placeholder implementation needs attention for some fields
The verification reveals that placeholders are only defined for some of the required fields:
tile_url: Has placeholder "please enter tile url"tile_zoomLevel: Has placeholder "please enter tile zoomLevel"tile_typeandtile_opacity: Missing placeholder definitions
🔗 Analysis chain
Verify placeholder implementation for specific fields.
The schema changes look good, but let's verify that placeholders are properly set for the fields mentioned in the PR objectives: tile_type, tile_url, tile_zoomLevel, and tile_opacity.
Also applies to: 47-47, 50-50
Let me search for placeholder definitions in the manifest files to complete the verification.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify placeholder text implementation in manifest files
# Expected: Find placeholder definitions for tile_type, tile_url, tile_zoomLevel, and tile_opacity
echo "Checking manifest files for placeholder definitions..."
rg -A 1 "id: (tile_type|tile_url|tile_zoomLevel|tile_opacity)" server/pkg/builtin/manifest*.yml
Length of output: 1109
Script:
#!/bin/bash
# Search for placeholder text definitions for the tile fields
rg -B 2 -A 2 "id: (tile_type|tile_url|tile_zoomLevel|tile_opacity)" server/pkg/builtin/manifest*.yml
Length of output: 2313
| @@ -380,6 +382,7 @@ func (i PropertySchemaField) schemaField(tf *TranslatedPropertySchemaField) (*pr | |||
| ID(property.FieldID(i.ID)). | |||
| Name(title). | |||
| Description(desc). | |||
| Placeholder(plac). | |||
There was a problem hiding this comment.
💡 Codebase verification
Missing test coverage for the new placeholder field
The test file convert_test.go contains schema field tests but lacks coverage for the new placeholder functionality. Please add test cases that:
- Verify the placeholder field is correctly set in the schema field builder
- Include both nil and non-nil placeholder scenarios
- Test placeholder translations if applicable
Example test case structure to add:
{
name: "with placeholder",
psg: PropertySchemaField{
ID: "field_with_placeholder",
Type: "string",
Placeholder: &str,
},
tl: &TranslatedPropertySchemaField{
Placeholder: i18n.String{"en": "Enter value"},
},
// ... expected result
}🔗 Analysis chain
LGTM: Proper integration with schema field builder.
The placeholder field is correctly integrated into the builder chain. Let's verify the test coverage for this new field.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check test coverage for the new placeholder field
# Test 1: Look for test cases covering the new placeholder field
rg -l "Placeholder\(" "server/pkg/plugin/manifest/**/*_test.go"
# Test 2: Check if there are any existing tests for schema field building
rg -A 5 "TestNewSchemaField" "server/pkg/plugin/manifest/**/*_test.go"
Length of output: 296
Script:
#!/bin/bash
# Let's first find the correct test file location
fd -e go -e test.go . server/pkg/plugin
# Then check for schema field tests and placeholder usage
rg -l "TestNewSchemaField|Placeholder\(" server/pkg/plugin
Length of output: 1432
Script:
#!/bin/bash
# Let's check the test coverage in convert_test.go specifically
rg -A 10 "func Test.*Schema.*Field" server/pkg/plugin/manifest/convert_test.go
# Also check for any placeholder-related test cases
rg -B 2 -A 5 "Placeholder\(" server/pkg/plugin/manifest/convert_test.go
Length of output: 375
Script:
#!/bin/bash
# Let's examine the full test file content to check test coverage
cat server/pkg/plugin/manifest/convert_test.go | grep -A 50 "func TestSchemaField"
Length of output: 1348
| @@ -1,6 +1,7 @@ | |||
| package manifest | |||
|
|
|||
| import "github.com/reearth/reearth/server/pkg/i18n" | |||
| // generated by "/var/folders/p1/v6rr29dn5yv4s1n244c1qxcr0000gp/T/go-build2967766196/b001/exe/schematyper -o pkg/plugin/manifest/schema_translation.go --package manifest --prefix Translation ./schemas/plugin_manifest_translation.json" -- DO NOT EDIT | |||
There was a problem hiding this comment.
Avoid including absolute paths in generated code comments
Including absolute paths in generated code comments can expose details about the build environment. Consider modifying the code generator to omit or anonymize the path.
| func (t *TranslatedPropertySchemaGroup) getOrCreateField(fid string) *TranslatedPropertySchemaField { | ||
| if fid == "" { | ||
| return nil | ||
| } | ||
| if t.Fields == nil { | ||
| t.Fields = map[string]*TranslatedPropertySchemaField{} | ||
| } |
There was a problem hiding this comment.
Fix potential nil pointer dereference in getOrCreateField function
In the function getOrCreateField, accessing t.Fields without checking if t is nil can lead to a nil pointer dereference when t is nil.
Update the function to check if t is nil before accessing its fields:
func (t *TranslatedPropertySchemaGroup) getOrCreateField(fid string) *TranslatedPropertySchemaField {
if fid == "" {
return nil
}
+ if t == nil {
+ return nil
+ }
if t.Fields == nil {
t.Fields = map[string]*TranslatedPropertySchemaField{}
}
if f := t.Fields[fid]; f != nil {
return f
}
f := &TranslatedPropertySchemaField{}
t.Fields[fid] = f
return f
}Ensure that t is not nil before proceeding with field access to prevent runtime panics.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (t *TranslatedPropertySchemaGroup) getOrCreateField(fid string) *TranslatedPropertySchemaField { | |
| if fid == "" { | |
| return nil | |
| } | |
| if t.Fields == nil { | |
| t.Fields = map[string]*TranslatedPropertySchemaField{} | |
| } | |
| func (t *TranslatedPropertySchemaGroup) getOrCreateField(fid string) *TranslatedPropertySchemaField { | |
| if fid == "" { | |
| return nil | |
| } | |
| if t == nil { | |
| return nil | |
| } | |
| if t.Fields == nil { | |
| t.Fields = map[string]*TranslatedPropertySchemaField{} | |
| } |
| func (t *TranslatedPropertySchema) getOrCreateGroup(gid string) *TranslatedPropertySchemaGroup { | ||
| if gid == "" { | ||
| return nil | ||
| } | ||
| if t == nil || *t == nil { | ||
| *t = TranslatedPropertySchema{} | ||
| } |
There was a problem hiding this comment.
Fix potential nil pointer dereference in getOrCreateGroup function
In the function getOrCreateGroup, the condition if t == nil || *t == nil can cause a nil pointer dereference when t is nil. Dereferencing *t when t is nil will result in a panic.
Please apply the following fix:
func (t *TranslatedPropertySchema) getOrCreateGroup(gid string) *TranslatedPropertySchemaGroup {
if gid == "" {
return nil
}
+ if t == nil {
+ return nil
+ }
if *t == nil {
*t = TranslatedPropertySchema{}
}
if g := (*t)[gid]; g != nil {
return g
}
g := &TranslatedPropertySchemaGroup{}
(*t)[gid] = g
return g
}Alternatively, ensure that t is not nil before calling this function or handle the nil case appropriately within the function.
Committable suggestion skipped: line range outside the PR's diff.
Overview
Add a placeholder to the field of the property.
What I've done
The settings are defined in the server’s manifest.yml file:
Note: server/pkg/builtin/manifest_ja.yml is for Japanese
The placeholder is applied under the fields tag:
... fields: - id: XXXXXXX ... + placeholder: please enter xxxxx - id: YYYYYYY ... + placeholder: please enter yyyyyIn this update, placeholders have been added for the following field IDs:
https://github.com/reearth/reearth-visualizer/blob/feat/placeholder-property-schema-field/server/pkg/builtin/manifest.yml#L326
What I haven't done
How I tested
https://github.com/reearth/reearth-visualizer/blob/feat/placeholder-property-schema-field/server/e2e/gql_scene_test.go#L15
Which point I want you to review particularly
Memo
It is necessary to update the web query.
web/src/services/gql/fragments/property.ts
const propertyFragment = gql` fragment PropertySchemaFieldFragment on PropertySchemaField { ... title description + placeholder translatedTitle(lang: $lang) translatedDescription(lang: $lang) + translatedPlaceholder(lang: $lang) prefix ... } `Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores