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

Enhancement of Misc/Set-BcContainerFeatureKeys.ps1 to be able to Enable Feature key for particular company. #3696

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

Conversation

KM-JAD
Copy link
Contributor

@KM-JAD KM-JAD commented Oct 2, 2024

  • In Set-BcContainerFeatureKeys is fixed creation of missing record in table "Tenant Feature Key". Also added option to use new parameter "EnableInCompany" which triggers activation/deactivation of Feature Key for given company name. This is usefull in case you would like to activate Feature e.g. before autotests. Without this setup Feature itself doesn't work in any company.
    (There is no activated any data upgrade. This must be initiated manually as usual, if it is really needed).

Fixes #3609

Mea Culpa - Despite the fact I confirmed it works correctly, I provided wrong code from incorrect branch in my repository, sorry for that. Never will happen again.
Added update of table [dbo].[Feature Data Update Status$63ca2fa4-4f03-4f2b-a480-172fef340d3f] as this table has to be updated as well to "mark" feature as activated/deactivated.
Optionally is possible to activate Feature for particular company. 
No data update is initiated there (main reason is to enable Feature for automated tests)
@KM-JAD KM-JAD requested a review from a team as a code owner October 2, 2024 12:44
@KM-JAD
Copy link
Contributor Author

KM-JAD commented Oct 2, 2024 via email

Misc/Set-BcContainerFeatureKeys.ps1 Outdated Show resolved Hide resolved
Misc/Set-BcContainerFeatureKeys.ps1 Outdated Show resolved Hide resolved
@freddydk
Copy link
Contributor

Please also add a line to the upper section of the release notes

KM-JAD and others added 2 commits October 11, 2024 07:49
Rebuilt code to be easy to understand it
@freddydk
Copy link
Contributor

freddydk commented Oct 11, 2024

I added another write-host - but I think there is still something wrong here...

With the change in this PR - we only ever change anything if the feature exists in the table Feature Data Update Status`$63ca2fa4-4f03-4f2b-a480-172fef340d3f.
In my database, that table contains:

[bcserver]: PS C:\Run> Invoke-Sqlcmd -Database 'default' -Query "SELECT ""Feature Key"" FROM [dbo].[Feature Data Update Status`$63ca2fa4-4f03-4f2b-a480-172fef340d3f]"

Feature Key

SalesPrices
ExtensibleInvoicePostingEngine
GLCurrencyRevaluation
SalesPrices

None of these are in the tenant Feature key table.
Meaning that you would never insert anything in the Tenant Feature Key or like.

@KM-JAD
Copy link
Contributor Author

KM-JAD commented Oct 11, 2024

  1. Regarding new Write-Host change, OK, why not...I wanted to use your original logic of messages to join both tables changes into one message. Frankly speaking, I do not undertstand why you have changed "else" with new "if" on lines 115-117, but it seems that logic is still the same :)

  2. Regarding missing records in table "Feature Data Update Status", well, the problem with Feature Management is a bit more complex.... There is an initial inconsistency in both tables "Tenant Feature Key" and "Feature Data Update Status" in original Cronus company provided within initial CRONUS database provided by MS.
    In standard way, all respective lines in table [Feature Data Update Status`$63ca2fa4-4f03-4f2b-a480-172fef340d3f] are created by each particular feature CU based on published event in 2610-"Feature Management". Such approach is quite universal through BC versions, which is fully OK for normal database usage, but not good for autotests in case you would like to run autotests within container built on last version of artifacts.
    Therefore we added to our auto-test process two additional steps (out of Container helper world)
    a) publishing page 2610 Feature Management as OData service (during extension for auto-tests installation process)
    b) calling of this published OData service to generate all lines within table "Feature Data Update Status" (powershell, response is thrown)
    In general, I didn't want to full rebuild your code....just wanted to make it working as initially intended.... :)

@freddydk
Copy link
Contributor

freddydk commented Oct 11, 2024

Regarding new Write-Host change, OK, why not...I wanted to use your original logic of messages to join both tables changes into one message. Frankly speaking, I do not undertstand why you have changed "else" with new "if" on lines 115-117, but it seems that logic is still the same :)

The reason for these dumps is really to see where things goes wrong if they go wrong - see which SQL statement causes the error - that is the reason.

But... - with your change - the only values that can be used with this function are these:

SalesPrices
ExtensibleInvoicePostingEngine
GLCurrencyRevaluation
SalesPrices

as that is the content of the data update table. I cannot call this with any other key - I don't assume that that is the way it should be?

I would like to understand how this function is supposed to work - after all - I am the one needing to support this going forward.

Currently - the creation of a new record in Tenant Feature Key also doesn't work - as the SQL Query contains [CRONUS] database name as well - and that fails always - so the [CRONUS] part needs to go away I guess.

I will try to describe how I think this function should work (after fixing the CRONUS item above).

  1. If called without a Companyname - it should create or update the feature key in [Tenant Feature Key] and set [Enabled] to 0|1 based on enabled.
  2. If called with a Companyname it should do (1) and also create or update the feature key in [Feature Data Update Status`$63ca2fa4-4f03-4f2b-a480-172fef340d3f] and set [Feature Status] to 0|1 based on enabled.

Is that the intended functionality?

@KM-JAD
Copy link
Contributor Author

KM-JAD commented Oct 11, 2024

as that is the content of the data update table. I cannot call this with any other key - I don't assume that that is the way it should be?

Yes, those four records are in database by default for company Cronus international, but you can create them also using configuration packages (instead of auto creation during opennig of page 2610)

Currently - the creation of a new record in Tenant Feature Key also doesn't work - as the SQL Query contains [CRONUS] database name as well - and that fails always - so the [CRONUS] part needs to go away I guess.

well, yes, [CRONUS] is there the same way, as there is Business Central instance "BC' :) . Works now perfectly for containers, but I think it will work w/o that as well.

I will try to describe how I think this function should work (after fixing the CRONUS item above).

  1. If called without a Companyname - it should create or update the feature key in [Tenant Feature Key] and set [Enabled] to 0|1 based on enabled.
  2. If called with a Companyname it should do (1) and also create or update the feature key in [Feature Data Update Status`$63ca2fa4-4f03-4f2b-a480-172fef340d3f] and set [Feature Status] to 0|1 based on enabled.

Is that the intended functionality?

yes, it is :)

Please note that standard BC database downloaded from MS repository has a bit inconsistent data records in both our tables.

@freddydk
Copy link
Contributor

well, yes, [CRONUS] is there the same way, as there is Business Central instance "BC' :) . Works now perfectly for containers, but I think it will work w/o that as well.

You specify -databaseName $databaseName and [CRONUS] - that doesn't work.
CRONUS is the databasename in a single tenant container - but the $databaseName is the name of a tenant.

@KM-JAD
Copy link
Contributor Author

KM-JAD commented Oct 14, 2024

You specify -databaseName $databaseName and [CRONUS] - that doesn't work.
CRONUS is the databasename in a single tenant container - but the $databaseName is the name of a tenant.

you are correct -> already fixed
Thanks for code revision :)

removed database name [CRONUS] from invoked SQL command on line 100.
New function for SaaS BC environment restart.
New function Restart-BcEnvironment added for SaaS BC environment restart.
New function to obtain SaaS BC sessions.
New function to stop and delete SaaS BC session.
Two new functions to obtain sessions and to delete session in SaaS BC .
Two new functions to obtain sessions and to delete session in SaaS BC.
@KM-JAD
Copy link
Contributor Author

KM-JAD commented Nov 7, 2024

Hi Freddy, is there anything missing from my side?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set-BcContainerFeatureKeys is not working
2 participants