Skip to content

[Ignore] atbt discerning Insert/Update#722

Open
7CMiguel wants to merge 5 commits intosqlkata:masterfrom
7CMiguel:issue-721
Open

[Ignore] atbt discerning Insert/Update#722
7CMiguel wants to merge 5 commits intosqlkata:masterfrom
7CMiguel:issue-721

Conversation

@7CMiguel
Copy link

@7CMiguel 7CMiguel commented Jul 23, 2024

#721 Related issue

Copy link
Contributor

@ahmad-moussawi ahmad-moussawi left a comment

Choose a reason for hiding this comment

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

I would suggest to not introduce a new attribute in this case,
we can use the same Ignore attribute and add optional parameters to select which operation should be ignored.

[Ignore(IgnoreOperation.OnInsert)] // [Ignore(IgnoreOperation.OnUpdate)] // [Ignore(IgnoreOperation.Always)] // the default option public string Name { get; set; }
@ahmad-moussawi
Copy link
Contributor

Nice feature!, I suggested to reuse the existing Ignore attribute, and please add test to cover this change

@7CMiguel
Copy link
Author

7CMiguel commented Mar 4, 2025

@ahmad-moussawi also about considerKeys, on Update/Insert, i was thinking that when is true it should be able to put the Where despite this attb being [Ignore], in my case we are using [Key, Ignore] for Ids, bcos they auto generate in db but for keys we want to insert we just avoid the [Ignore], but i need to put myself for the update.Where(keys, keys.value) more than 1 where if needed for complex keys, but if we would leave this outside can be usefull. What d u think.

Instead of
if (property.GetCustomAttribute(typeof(IgnoreAttribute)) != null) { continue; }
First and then
if (considerKeys && colAttr != null) { if ((colAttr as KeyAttribute) != null) { this.Where(name, value); } }
Put the consider keys + Where first and then the continue

@7CMiguel
Copy link
Author

7CMiguel commented Mar 7, 2025

@ahmad-moussawi changes aplied + UT to check the ignore on diferent cases.

@7CMiguel 7CMiguel requested a review from ahmad-moussawi April 2, 2025 12:49
@7CMiguel
Copy link
Author

@ahmad-moussawi The Pr its ready from a time, check it please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants