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

#1978 HasDatabaseUpgradeTag #2061

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

TKapitan
Copy link
Contributor

@TKapitan TKapitan commented Sep 18, 2024

Summary

Adds HasDatabaseUpgradeTag() procedure to make the setters and getters the same.

Work Item(s)

Fixes #1978

Fixes AB#550241

@TKapitan TKapitan requested a review from a team as a code owner September 18, 2024 14:48
@github-actions github-actions bot added AL: System Application From Fork Pull request is coming from a fork labels Sep 18, 2024
Co-authored-by: Natalie Karolak, MVP <[email protected]>
@JesperSchulz JesperSchulz added the Linked Issue is linked to a Azure Boards work item label Sep 24, 2024
@JesperSchulz JesperSchulz enabled auto-merge (squash) September 24, 2024 10:42
@github-actions github-actions bot added this to the Version 26.0 milestone Sep 24, 2024
@JesperSchulz JesperSchulz added the Integration GitHub request for Integration area label Sep 24, 2024
/// </summary>
/// <param name="Tag">Tag code to check</param>
/// <returns>True if the Tag with the given code exists.</returns>
procedure HasDatabaseUpgradeTag(Tag: Code[250]): Boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

So the idea was to use SetUpgradeTag and HasUpgradeTag only, the code will find out from the context if the tag is per database or per company. Approving since we already have the SetDatabaseUpgradeTag, which is wrongly named IMO. It should be PerDatabaseUpgradeTag

@@ -43,6 +43,16 @@ codeunit 9999 "Upgrade Tag"
exit(UpgradeTagImpl.HasUpgradeTag(Tag, TagCompanyName));
end;

/// <summary>
/// Verifies if the database upgrade tag exists.
Copy link
Contributor

Choose a reason for hiding this comment

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

per-databse

/// <summary>
/// Verifies if the database upgrade tag exists.
/// </summary>
/// <param name="Tag">Tag code to check</param>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment that the developer can use HasUpgradeTag if it is called from the upgrade context? If it is called from upgrade context e.g. OnUpgradePerDatabase we will automatically use perdatabase check and similar for percompany?

Nothing wrong with using this one directly but it could be confusing if someone is reading the existing code.

@JesperSchulz
Copy link
Contributor

@TKapitan, should we try to reel this one in? Not much pending, the way I see it. Nikola even approved.

@JesperSchulz JesperSchulz self-assigned this Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AL: System Application From Fork Pull request is coming from a fork Integration GitHub request for Integration area Linked Issue is linked to a Azure Boards work item
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: UpgradeTag does not have a procedure for database upgrade tags
5 participants