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.

Sign up for GitHub

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

Jump to bottom

#1978 HasDatabaseUpgradeTag #2061

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

#1978 HasDatabaseUpgradeTag #2061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f929424
HasDatabaseUpgradeTag
TKapitan Sep 18, 2024
42eff37
Address code review comments.
JesperSchulz Sep 24, 2024
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
10 changes: 10 additions & 0 deletions src/System Application/App/Upgrade Tags/src/UpgradeTag.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
/// <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.

/// <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

begin
exit(UpgradeTagImpl.HasDatabaseUpgradeTag(Tag));
end;

/// <summary>
/// Sets the upgrade tag for database upgrades.
/// </summary>
Expand Down
5 changes: 5 additions & 0 deletions src/System Application/App/Upgrade Tags/src/UpgradeTagImpl.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ codeunit 9996 "Upgrade Tag Impl."
exit(HasUpgradeTag(Tag, CopyStr(CompanyName(), 1, MaxStrLen(ConstUpgradeTags.Company))));
end;

procedure HasDatabaseUpgradeTag(Tag: Code[250]): Boolean
begin
exit(HasUpgradeTag(Tag, ''));
end;

procedure HasUpgradeTag(Tag: Code[250]; TagCompanyName: Code[30]): Boolean
var
UpgradeTags: Record "Upgrade Tags";
Expand Down
Loading