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

Import Columns Bug 2427 #2707

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 56 additions & 33 deletions src/app/core/basic-datatypes/entity/entity.datatype.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,10 @@
/*
* This file is part of ndb-core.
*
* ndb-core is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* ndb-core is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with ndb-core. If not, see <http://www.gnu.org/licenses/>.
*/

import { Injectable } from "@angular/core";
import { StringDatatype } from "../string/string.datatype";
import { EntitySchemaField } from "../../entity/schema/entity-schema-field";
import { EntityMapperService } from "../../entity/entity-mapper/entity-mapper.service";
import { ColumnMapping } from "../../import/column-mapping";
import { EntityActionsService } from "../../entity/entity-actions/entity-actions.service";

/**
* Datatype for the EntitySchemaService to handle a single reference to another entity.
* Stored as simple id string.
*
* For example:
*
* `@DatabaseField({dataType: 'entity', additional: 'Child'}) relatedEntity: string;`
*/
Comment on lines -25 to -32
Copy link
Member

Choose a reason for hiding this comment

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

don't remove existing code documentation please

@Injectable()
export class EntityDatatype extends StringDatatype {
static override dataType = "entity";
Expand All @@ -46,17 +21,65 @@ export class EntityDatatype extends StringDatatype {
super();
}

override importMapFunction(
/**
* Maps a value from an import to an actual entity in the database by comparing the value with the given field of entities.
* Handles type conversion between numbers and strings to improve matching.
*
* @param val The value from an import that should be mapped to an entity reference
* @param schemaField The config defining details of the field that will hold the entity reference after mapping
* @param additional The field of the referenced entity that should be compared with the val
* @returns Promise resolving to the ID of the matched entity or undefined if no match is found
*/
override async importMapFunction(
val: any,
schemaField: EntitySchemaField,
additional?: any,
) {
if (!additional) {
return Promise.resolve(undefined);
additional?: string,
): Promise<string | undefined> {
// If no additional field is specified or value is null/undefined, return undefined
if (!additional || val == null) {
return undefined;
}

// Convert value to string or number for flexible matching
Copy link
Member

Choose a reason for hiding this comment

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

Have you worked with (jasmine) unit tests before? It would be good to write some test cases for the EntityDatatype's importMapFunction

const normalizedVal = this.normalizeValue(val);

try {
// Load all entities of the specified type
const entities = await this.entityMapper.loadType(schemaField.additional);

// Find the first entity where the specified field matches the normalized value
const matchedEntity = entities.find((entity) => {
const entityFieldValue = this.normalizeValue(entity[additional]);
return entityFieldValue === normalizedVal;
});

// Return the ID of the matched entity or undefined
return matchedEntity?.getId();
} catch (error) {
console.error("Error in EntityDatatype importMapFunction:", error);
Copy link
Member

Choose a reason for hiding this comment

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

We have a special Logging service that we use instead of console.log in order to add some functionality like remote monitoring of errors. Please have a look here how to use that: https://aam-digital.github.io/ndb-core/documentation/additional-documentation/how-to-guides/log-errors.html

If you get stuck or have questions on this, do let us know.

return undefined;
}
return this.entityMapper
.loadType(schemaField.additional)
.then((res) => res.find((e) => e[additional] === val)?.getId());
}

/**
* Normalize a value for comparison, converting to string or number as appropriate
* @param val The value to normalize
* @returns Normalized value (string or number)
*/
private normalizeValue(val: any): string | number {
// If the value is already a string or number, return it
if (typeof val === "string" || typeof val === "number") {
return val;
}

// Try to convert to number if possible
const numVal = Number(val);
if (!isNaN(numVal)) {
return numVal;
}

// Convert to string as a fallback
return String(val);
}

override importIncompleteAdditionalConfigBadge(col: ColumnMapping): string {
Expand Down
21 changes: 2 additions & 19 deletions src/app/core/entity/default-datatype/default.datatype.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,3 @@
/*
* This file is part of ndb-core.
*
* ndb-core is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* ndb-core is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with ndb-core. If not, see <http://www.gnu.org/licenses/>.
*/

import { EntitySchemaField } from "../schema/entity-schema-field";
import { Entity } from "../model/entity";
import { ColumnMapping } from "../../import/column-mapping";
Expand Down Expand Up @@ -103,8 +86,8 @@ export class DefaultDatatype<EntityType = any, DBType = any> {
async importMapFunction(
val: any,
schemaField: EntitySchemaField,
additional?: any,
): Promise<EntityType | EntityType[]> {
additional?: string,
Copy link
Member

Choose a reason for hiding this comment

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

as this base class gets extended by various implementation, additional can have different types here and cannot be forced to string always. Leave this as "any" here. In the EntityDatatype class, you should be able to change the parameter to have type "string" in that specific implementation.

Suggested change
additional?: string,
additional?: any,

): Promise<EntityType | EntityType[] | undefined> {
if (schemaField.isArray) {
return asArray(val).map((v) =>
this.transformToObjectFormat(v, schemaField),
Expand Down