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

NotBlank = TRUE is not working when you have a PK field which is based on a number series #620

Closed
janvtBE opened this issue May 21, 2024 · 12 comments · Fixed by #661
Closed
Labels
bug Something isn't working Resolved

Comments

@janvtBE
Copy link

janvtBE commented May 21, 2024

Hi,

We had some issues with rule LC0013: https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0013.

When you have a single PK field which is Text or Code and this field is filled in by a number serie, the NotBlank = TRUE is causing the following error:

image

How we interpret this: the NotBlank property is executed first before the OnInsert trigger, where the code is to fill in a new number of the coupled number serie.

Has anyone had the same issue with this and how did you handled this? Putting the NotBlank to FALSE or something else? :)

Second question: is it an option to create a rule for this? Of course, how do you know when there is a number serie coupled to this field...

All input / other options are welcome :)

Thanks

@Arthurvdv
Copy link
Collaborator

Arthurvdv commented May 23, 2024

@janvtBE thank you for reporting this.

Could it be that you table didn't include the 'No. Series' pattern?

@pri-kise
Copy link

We had the same issue on a customer project for those tables and disabled the LinterCop rule for those specific fields.

@Arthurvdv Arthurvdv added the question Further information is requested label May 23, 2024
@Arthurvdv
Copy link
Collaborator

We had the same issue on a customer project for those tables and disabled the LinterCop rule for those specific fields.

Strange, because the rule should consider if the the 'No. Series' pattern is there and then will not be raised? 🤔

@janvtBE
Copy link
Author

janvtBE commented May 24, 2024

@Arthurvdv the field "No. Series" was there, but had no tablerelation with "No. Series" table. But also after adding this to the field, it still raised the error:

image

@Arthurvdv Arthurvdv added the bug Something isn't working label May 25, 2024
@Arthurvdv
Copy link
Collaborator

Seems that we have a bug, I'll have a look into this!

@Arthurvdv
Copy link
Collaborator

It seems I can't reproduce this problem and could use some help.

When I set the 'TableRelation = "No. Series";' to a field on the table, the diagnostic of the LC0013 isn't raised.

table 50100 MyTable
{
    fields
    {
        field(1; "No."; Code[20])
        {
            Caption = 'No.';
        }

        field(100; "No. Series"; Code[20])
        {
            Caption = 'No. Series';
            TableRelation = "No. Series";
        }
    }

    keys
    {
        key(PK; "No.")
        {
            Clustered = true;
        }
    }
}

@Arthurvdv Arthurvdv added help wanted Extra attention is needed and removed question Further information is requested labels May 25, 2024
@NKarolak
Copy link

NKarolak commented Jun 20, 2024

@Arthurvdv
I'm seeing LC0013 now suddenly as well, and I'm not aware of a recent code change on my side, unless it's the added AllowInCustomizations below - added due to LC0035. @janvtBE is using it as well!

Mind that my "No. Series" field looks slightly different, because IMHO the PK fields should always be set in a TableRelation to feed Where Used (new linter rule? ;-)):

        field(107; "No. Series"; Code[20])
        {
            AllowInCustomizations = Always; // <-- added recently due to LC0035
            Caption = 'No. Series';
            Editable = false;
            TableRelation = "No. Series".Code;
        }

@NKarolak
Copy link

@Arthurvdv I've just tested it: No, AllowInCustomizations has no influence.
But my TableRelation = "No. Series".Code; does have it!
If I set it to TableRelation = "No. Series";, then LC0013 vanishes :-)
Weird that the warning appeared so late without a code change on my side.

But it looks this is not the cause on @janvtBE's side, as per his screenshot.

@Arthurvdv Arthurvdv added part of upcoming release and removed help wanted Extra attention is needed labels Jun 30, 2024
@Arthurvdv
Copy link
Collaborator

Thank you @NKarolak, you were right on including the PK field (Code) to the TableRelation would trigger a false positive.

Determining the table TableRelation to the "No. Series" includes some string comparison with quoting and unquoting string value. Could be that there's been some changes under the hood which influenced the behavior on this. I've changed the logic on the rule on this to provide a better handeling on this.

I'll create two new idea's in the Discussions based of the information in here:

  • NotBlank on single field PKs false if there's a TableRelation = "No. Series" present on the table
  • The PK fields should always be set in a TableRelation (to feed Where Used)

@Arthurvdv
Copy link
Collaborator

The version v0.30.28 of the LinterCop is now the latest release. Could you verify if this is now working as expected? If this is the case, you may close this issue.

@NKarolak
Copy link

I'm not the issue creator - but for me, it's solved. Thanks!

@janvtBE
Copy link
Author

janvtBE commented Jul 17, 2024

Thanks @Arthurvdv for the fix: https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0067. I'll close this issue as resolved!

@janvtBE janvtBE closed this as completed Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Resolved
Projects
None yet
4 participants