cortezaproject / corteza

Low-code platform
https://cortezaproject.org
Apache License 2.0
1.61k stars 371 forks source link

When I try to create records that have a single digit record selector field I get an error #1418

Closed m4thfr34k closed 9 months ago

m4thfr34k commented 1 year ago

Is there an existing issue for this?

Version of Corteza

No response

Current Behavior

Unable to create records associated to other tables/modules when the other table has a single digit id. The field type is a number but because the id is < 10 we receive the following error:

Error: Could not create record: invalid reference format

I think I’ve tracked this error down to the RecordValueSanitization function in the record.go file.

It looks like it’s checking for a numeric using the regex ^1-9$ which fails for single digit numbers.

There is a Numeric regex in patterns.go that works for single digit numbers in addition to anything >= 10 Numeric string = “^[0-9]+$”

Expected Behavior

Should be able to create records that have a field, type Record selector, with a single digit value, value < 10.

Steps To Reproduce

Create a module connected to an external table that has at least one record where the record ID < 10. Create a second module that has a field that is a record selector type where the Module name is set to the module above. Create a record in the second module and populate that record selector field with a record from the first where the ID < 10. Click save and receive error

Environment and versions

Corteza version: 2023.3.4

Anything else?

Link to discussion in forum: https://forum.cortezaproject.org/t/record-create-failure-for-single-digit-record-ids/1548

tjerman commented 1 year ago

Have you checked if changing that condition brings implications to anything else? I don't think there is anything wrong with changing those constraints, but I'm a bit spooked at what else would crash so a bunch of testing would need to be done.

IIRC there used to be a condition (somewhere) that all the IDs had to be of length x to further limit it to sonyflake unit values (but I could be wrong so don't quote me on it)

There were some ideas/thoughts we should support different identifier types within DAL for when you connect to external sources, but nothing concrete as of now.


As a workaround

As a workaround you could consider altering your DB rows to add x to all IDs; for example, 10 so the condition would pass with no questions and minimal issues (since all IDs would just be displaced by x); is that an option for you until we do some proper fix?

m4thfr34k commented 1 year ago

Manually adding 10 to all IDs in all tables we would connect to is not a viable option for us. These tables have a fair amount of relationships to other tables and since we are talking about the ID field itself, which would be the foreign key in other tables, that would not be a road we would prefer to go down.

I understand sonyflake values and them being used for the ID field in Corteza created tables but this error presents itself with other/external/non-Corteza created tables. This particular piece of code wants a number in the ID field and single digit IDs are failing due to this issue.

I didn't see any comments in the code, where this is checked, that suggests the original intent for this specific check was for single digit numbers to fail the 'IsNumeric' check. Any/all double digit numbers pass, like 10, but a double digit number isn't a valid sonyflake ID so I'm not sure that sonyflake requirements have anything to do with this particular code/check.

tjerman commented 1 year ago

Thought so. Probably just a lapsus on our part but it'll still need a bunch of testing

dcharpen commented 11 months ago

Is there any other information needed for this issue to be worked?

tjerman commented 10 months ago

@KinyaElGrande looks like it'll most likely be a regex issue (see + instead of * in te first search result).

Image

Please do check if this is the only thing that needs to change; think this should be it but I'm not entirely sure

dcharpen commented 9 months ago

Is there some workaround for this besides us altering our DB rows to add x to all IDs and making the same change on all tables that use that ID as a foreign key?

This stops the creation of records associated to other tables/modules when the other table has a single digit id. There are plenty of options for using external tables with Corteza which would be impacted by this issue.

m4thfr34k commented 9 months ago

There is a Numeric regex in patterns.go that works for single digit numbers in addition to anything >= 10 Numeric string = “^[0-9]+$”

KinyaElGrande commented 9 months ago

Hi @m4thfr34k and @dcharpen the issue has now been resolved with https://github.com/cortezaproject/corteza/pull/1614. You are now able to reference single-digit record IDs using record selector without any error.