Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request is a work-in-progress (WIP) migration from using node UUIDs to node internal IDs (IIds) as the primary identifier for nodes in the Arena survey platform. The migration affects multiple layers of the application including the database schema, server-side repositories and managers, frontend state management, and test files.
Changes:
- Added local dependency on
@openforis/arena-corepackage containing shared utilities for the migration - Updated database schema to use internal IDs (
i_id,p_i_id) alongside UUIDs for node identification - Modified server-side repositories, managers, and APIs to work with internal IDs
- Updated frontend state management and components to use internal IDs
- Modified test utilities and integration tests to support the new identifier system
Reviewed changes
Copilot reviewed 59 out of 61 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Added local dependency for @openforis/arena-core package |
| package.json | Added local arena-core dependency |
| core/objectUtils.js | Added getIId, getParentInternalId, and toIIdIndexedObj utility functions |
| core/record/node.js | Added iId and pIId keys and corresponding getter functions |
| core/record/nodeKeys.js | Changed nodeUuid key to nodeIId |
| core/record/record.js | Exported getNodeByInternalId alongside getNodeByUuid |
| core/record/_record/recordReader.js | Updated to import and export getNodeByInternalId |
| core/record/_record/recordUpdater.js | Updated documentation formatting |
| core/record/_record/recordsCombiner.js | Migrated functions to use IId instead of UUID for node lookups |
| core/record/_record/recordNodesUpdater.js | Updated parameter names from nodeUuids to nodeIIds |
| core/record/recordFile.js | Changed nodeUuid property to nodeIId |
| core/record/recordValidationReportItem.js | Updated to use nodeIId key |
| common/objectUtils.js | Added iId and pIId keys |
| common/activityLog/activityLog.js | Added nodeIId content key |
| common/model/db/tables/table.js | Added iId to common column set |
| common/model/db/tables/dataNodeDef/table.js | Updated table schema to use i_id, p_i_id columns and composite primary keys |
| common/model/db/tables/dataNodeDef/dataColProps.js | Updated entity value processor to return IId |
| server/modules/surveyRdb/schemaRdb/*.js | Updated RDB view schemas to use nodeIId |
| server/modules/surveyRdb/repository/*.js | Updated RDB repository queries to use internal IDs |
| server/modules/surveyRdb/service/*.js | Updated uniqueness validation to use IId |
| server/modules/record/repository/nodeRepository.js | Updated CRUD operations to use i_id column and internal IDs |
| server/modules/record/manager/*.js | Updated record managers to use internal IDs |
| server/modules/record/api/recordApi.js | Updated DELETE endpoint to accept nodeIId parameter |
| server/modules/record/service/recordService.js | Updated service methods to use nodeIId |
| server/modules/survey/service/recordCheckJob.js | Updated to index nodes by IId |
| server/modules/mobile/service/*.js | Updated mobile import to use IId in logging |
| server/modules/dataImport/service/*.js | Updated data import jobs to use IId tracking |
| server/modules/collectImport/service/*.js | Updated Collect import to use nodeIId |
| webapp/store/ui/surveyForm/*.js | Updated form state and actions to use IId |
| webapp/store/ui/record/hooks/index.js | Updated hooks to use IId |
| webapp/store/ui/record/actions/*.js | Updated record actions to use IId |
| webapp/components/survey/SurveyForm/nodeDefs/components/*.js | Updated components to use IId for keys and node selection |
| webapp/service/api/data/index.js | Updated file URL generation to use IId |
| test/utils/*.js | Updated test utilities to use IId |
| test/integration/tests/*.js | Updated integration tests to use IId |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
SteRiccio
changed the title
SteRiccio
marked this pull request as ready for review
September 1, 2026 09:43
# Conflicts: # .yarn/install-state.gz # common/model/db/tables/dataNodeDef/table.js # core/objectUtils.js # core/record/_record/recordNodesUpdater.js # core/survey/surveyFile.js # package.json # server/modules/collectImport/service/collectImport/dataImportJobs/collectAttributeValueExtractor.js # server/modules/dataImport/service/DataImportJob/EntitiesDeleteJob.js # server/modules/mobile/service/arenaMobileDataImport/jobs/recordsImportJob.js # server/modules/record/api/recordApi.js # server/modules/record/manager/_recordManager/nodeCreationManager.js # server/modules/record/manager/_recordManager/nodeUpdateManager.js # server/modules/record/manager/recordManager.js # server/modules/record/service/recordService.js # server/modules/record/service/recordsCloneJob.js # server/modules/record/service/update/thread/recordsUpdateThread.js # server/modules/survey/service/recordCheckJob.js # server/modules/surveyRdb/repository/dataTableInsertRepository.js # server/modules/surveyRdb/repository/dataTableUpdateRepository.js # server/modules/surveyRdb/repository/dataView/create.js # server/modules/surveyRdb/service/surveyRdbCreationJob/recordsUniquenessValidationJob.js # test/e2e/tests/_surveyExport/records.js # webapp/components/survey/NodeDefsSelector/NodeDefTreeSelect/useBuildEntityTreeData.js # webapp/components/survey/Record/RecordNoHeader.js # webapp/components/survey/SurveyForm/nodeDefs/components/types/nodeDefEntityFormNodeSelect.js # webapp/components/survey/SurveyForm/nodeDefs/components/types/nodeDefEntityTableRows.js # webapp/store/ui/record/hooks/index.js # webapp/store/ui/surveyForm/state.js # webapp/views/App/views/Data/MapView/MapView.js # yarn.lock
…Mode data view read The uuid->iId rename updated node/select.js's getSelect() to destructure parentIId instead of parentUuid, but this one call site (the RDB "edit mode" per-column LATERAL join) was never updated to match. Since the key no longer matched, the parent-scoping condition was silently never added - either returning unscoped node values from the wrong parent, or (for single-entity nested columns) no rows at all, since the resulting condition compared against a literal null. The value being passed was already correctly i_id-based (ColumnNodeDef.getColumnName appends _i_id for entity defs) - only the param key was stale. No test currently exercises this path.
…efore the node internal-id migration Detects the legacy uuid/parentUuid node linkage and converts it to the current iId/pIId shape before handing the record to the rest of the (iId-only) record model, so old mobile app builds can keep uploading data after the server switches to internal ids. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…to every zip consumer The previous fix only converted legacy-format records in the arena-mobile live-sync import job. Restoring an old Arena backup, or previewing a mobile upload before importing it, read records through the same ArenaSurveyFileZip.getRecord accessor and would hit the same break (iId-only node persistence receiving an undefined internal id). Moving the conversion into getRecord itself covers all three call sites from one place instead of duplicating the check at each of them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…a minimum-version map Reads the arena-mobile app version from info.json in the uploaded zip and rejects the import if it's below the minimum this running arena version requires. The compatibility map is empty for now (no arena release has needed one yet) - it's meant to be filled in when a future client-facing breaking change, e.g. the node internal-id migration, needs arena-mobile to be updated in step. Uses arena-core's own Versions utility for comparison (handles an optional "v" prefix and a missing patch segment) rather than a hand-rolled comparator. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e.uuid fallout Paired with the arena-server migration that drops node.node_def_uuid in favor of node.node_def_id (bigint, saves 16 bytes/row + index vs uuid on large surveys). nodeDefUuid stays the in-memory/API field everywhere; nodeRepository's shared select query now joins node_def to expose it, inserts resolve it via a raw subquery/survey lookup, and the handful of raw WHERE/UPDATE/RDB-view queries that filtered node_def_uuid directly (nodeRepository, massiveUpdateNodes, common/model/db/tables/node, nodeHierarchyDisaggregatedViewRepository) do the same. insertAllQuery's positional column mapping is fixed to support pg-promise column descriptor objects (needed for the raw node_def_id subquery column). Also fixes two remaining spots left broken by the earlier node i_id migration (which already dropped node.uuid/parent_uuid): validationReportRepository selected node.uuid into an unused nodeUuid field (removed, nothing read it); dataView's fetchRecordsCountByRootNodesValue aggregated node.uuid into nodesKeyUuids for record-uniqueness validation field keys, which recordUniquenessValidator's sibling validation path already keys by node i_id - switched to match (nodesKeyIIds). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tead of a local copy core/record/recordNodeIdMigration.ts duplicated logic that now lives in arena-core as RecordFixer.isLegacyNodeFormat/initInternalIds (aligned on the lastNodeInternalId field name so it's a drop-in replacement for arena's own record model). arenaSurveyFileZip.js and the node-id-migration merge test now call the shared implementation; the local copy and its standalone test are removed. The migrated shape now leaves a root node's pIId unset rather than explicitly null, matching arena-core's own NodeFactory convention - nothing in arena reads pIId with a strict null check, so arenaSurveyFileZipRecordMigration.test.ts's expectations are updated to match. Bumps the installed @openforis/arena-core to pick up the new RecordFixer exports. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.