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

[AC 1427]Add New Organisation Properties Update DB Objects and create migration #2980

Merged
merged 21 commits into from
Jun 12, 2023
Merged

[AC 1427]Add New Organisation Properties Update DB Objects and create migration #2980

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1ebeeda
Add new properties to organization
cyprain-okeke May 30, 2023
6f00a46
Add new properties to organization
cyprain-okeke May 30, 2023
c497ca2
Create migration
cyprain-okeke May 30, 2023
66cfeaa
Add the columns to the view
cyprain-okeke May 30, 2023
de69dde
Fix the syntax error
cyprain-okeke May 30, 2023
7e9a68f
Change the namespaces
cyprain-okeke May 30, 2023
0590fde
Remove the comma on the stripe file
cyprain-okeke May 30, 2023
07224ec
Merge branch 'master' into ac-1427/add_new_org_properties_update_db_o…
cyprain-okeke May 30, 2023
b2371a1
Remove the nulls
cyprain-okeke May 31, 2023
2ff0a89
Merge branch 'master' into ac-1427/add_new_org_properties_update_db_o…
cyprain-okeke May 31, 2023
2c67ece
Resolving the PR comments
cyprain-okeke Jun 1, 2023
e5de4fc
Merge branch 'ac-1427/add_new_org_properties_update_db_objects_and_cr…
cyprain-okeke Jun 1, 2023
d868ffc
Merge branch 'master' into ac-1427/add_new_org_properties_update_db_o…
cyprain-okeke Jun 1, 2023
a658e77
Add a refresh for OrganizationView
cyprain-okeke Jun 2, 2023
aedef82
Merge branch 'master' into ac-1427/add_new_org_properties_update_db_o…
cyprain-okeke Jun 2, 2023
06a10b2
Remove the True default values
cyprain-okeke Jun 5, 2023
d97fec3
Merge branch 'ac-1427/add_new_org_properties_update_db_objects_and_cr…
cyprain-okeke Jun 5, 2023
7007fdf
Merge branch 'master' into ac-1427/add_new_org_properties_update_db_o…
cyprain-okeke Jun 5, 2023
b43898c
Resolve the comments
cyprain-okeke Jun 5, 2023
1455a30
Merge branch 'ac-1427/add_new_org_properties_update_db_objects_and_cr…
cyprain-okeke Jun 5, 2023
621503e
Merge branch 'master' into ac-1427/add_new_org_properties_update_db_o…
cyprain-okeke Jun 12, 2023
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/Api/Models/Response/Organizations/OrganizationResponseModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ public OrganizationResponseModel(Organization organization, string obj = "organi
UseCustomPermissions = organization.UseCustomPermissions;
SelfHost = organization.SelfHost;
HasPublicAndPrivateKeys = organization.PublicKey != null && organization.PrivateKey != null;
UsePasswordManager = organization.UsePasswordManager;
SmSeats = organization.SmSeats;
SmServiceAccounts = organization.SmServiceAccounts;
MaxAutoscaleSmSeats = organization.MaxAutoscaleSmSeats;
MaxAutoscaleSmServiceAccounts = organization.MaxAutoscaleSmServiceAccounts;
}

public string Id { get; set; }
Expand Down Expand Up @@ -81,6 +86,11 @@ public OrganizationResponseModel(Organization organization, string obj = "organi
public bool UseCustomPermissions { get; set; }
public bool SelfHost { get; set; }
public bool HasPublicAndPrivateKeys { get; set; }
public bool UsePasswordManager { get; set; } = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This default initialization value seems to be ignored since it's already initialized by the constructor. What's the reason for including it here? Perhaps just to document the intended default value?

Copy link
Member

@eliykat eliykat Jun 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That's a good point, what do you think @cyprain-okeke?

Copy link
Member

@eliykat eliykat Jun 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Cy's response (@cyprain-okeke looks like you accidentally edited my comment instead, so I'm just posting this separately for you)

I agree with you but The = true part of the declaration assigns a default value to the property. It means that if no value is explicitly assigned to UsePasswordManager, it will default to true.

So, in this case, the property UsePasswordManager will default to true if no value is explicitly assigned to it.

Copy link
Member

Choose a reason for hiding this comment

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

Yes but it's a non-nullable bool, so it will always have a value assigned in the constructor, and this default assignment will never be used. (Please correct me if I'm wrong!)

public int? SmSeats { get; set; }
public int? SmServiceAccounts { get; set; }
public int? MaxAutoscaleSmSeats { get; set; } = null;
public int? MaxAutoscaleSmServiceAccounts { get; set; } = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Specifying null as the default value is redundant here given the datatype is already an int?

Copy link
Member

@eliykat eliykat Jun 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The default value is 0, not null: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/default-values

I agree with your comment above though that these default assignments are probably not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This null has been removed. I agree with Conner

Copy link
Member

Choose a reason for hiding this comment

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

Oops sorry, the default value is indeed null for a nullable type. d'oh.

@cyprain-okeke This change is not showing, please double check that you've removed these assignments and pushed to remote.

}

public class OrganizationSubscriptionResponseModel : OrganizationResponseModel
Expand Down
5 changes: 5 additions & 0 deletions src/Core/Entities/Organization.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ public class Organization : ITableObject<Guid>, ISubscriber, IStorable, IStorabl
public int? MaxAutoscaleSeats { get; set; } = null;
public DateTime? OwnersNotifiedOfAutoscaling { get; set; } = null;
public OrganizationStatusType Status { get; set; }
public bool UsePasswordManager { get; set; } = true;
eliykat marked this conversation as resolved.
Show resolved Hide resolved
public int? SmSeats { get; set; }
public int? SmServiceAccounts { get; set; }
public int? MaxAutoscaleSmSeats { get; set; }
public int? MaxAutoscaleSmServiceAccounts { get; set; }

public void SetNewId()
{
Expand Down
3 changes: 3 additions & 0 deletions src/Core/Models/Data/Organizations/OrganizationUsers/OrganizationUserOrganizationDetails.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,7 @@ public class OrganizationUserOrganizationDetails
public DateTime? FamilySponsorshipValidUntil { get; set; }
public bool? FamilySponsorshipToDelete { get; set; }
public bool AccessSecretsManager { get; set; }
public bool UsePasswordManager { get; set; }
public int? SmSeats { get; set; }
public int? SmServiceAccounts { get; set; }
}
3 changes: 3 additions & 0 deletions ...ture.EntityFramework/Repositories/Queries/OrganizationUserOrganizationDetailsViewQuery.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ from os in os_g.DefaultIfEmpty()
FamilySponsorshipToDelete = os.ToDelete,
FamilySponsorshipValidUntil = os.ValidUntil,
AccessSecretsManager = ou.AccessSecretsManager,
UsePasswordManager = o.UsePasswordManager,
SmSeats = o.SmSeats,
SmServiceAccounts = o.SmServiceAccounts
};
return query;
}
Expand Down
21 changes: 18 additions & 3 deletions src/Sql/dbo/Stored Procedures/Organization_Create.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@
@UseScim BIT = 0,
@UseCustomPermissions BIT = 0,
@UseSecretsManager BIT = 0,
@Status TINYINT = 0
@Status TINYINT = 0,
@UsePasswordManager BIT = 1,
@SmSeats INT = null,
@SmServiceAccounts INT = null,
@MaxAutoscaleSmSeats INT= null,
@MaxAutoscaleSmServiceAccounts INT = null
AS
BEGIN
SET NOCOUNT ON
Expand Down Expand Up @@ -96,7 +101,12 @@ BEGIN
[UseScim],
[UseCustomPermissions],
[UseSecretsManager],
[Status]
[Status],
[UsePasswordManager],
[SmSeats],
[SmServiceAccounts],
[MaxAutoscaleSmSeats],
[MaxAutoscaleSmServiceAccounts]
)
VALUES
(
Expand Down Expand Up @@ -145,6 +155,11 @@ BEGIN
@UseScim,
@UseCustomPermissions,
@UseSecretsManager,
@Status
@Status,
@UsePasswordManager,
@SmSeats,
@SmServiceAccounts,
@MaxAutoscaleSmSeats,
@MaxAutoscaleSmServiceAccounts
)
END
14 changes: 12 additions & 2 deletions src/Sql/dbo/Stored Procedures/Organization_Update.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@
@UseScim BIT = 0,
@UseCustomPermissions BIT = 0,
@UseSecretsManager BIT = 0,
@Status TINYINT = 0
@Status TINYINT = 0,
@UsePasswordManager BIT = 1,
@SmSeats INT = null,
@SmServiceAccounts INT = null,
@MaxAutoscaleSmSeats INT = null,
@MaxAutoscaleSmServiceAccounts INT = null
AS
BEGIN
SET NOCOUNT ON
Expand Down Expand Up @@ -96,7 +101,12 @@ BEGIN
[UseScim] = @UseScim,
[UseCustomPermissions] = @UseCustomPermissions,
[UseSecretsManager] = @UseSecretsManager,
[Status] = @Status
[Status] = @Status,
[UsePasswordManager] = @UsePasswordManager,
[SmSeats] = @SmSeats,
[SmServiceAccounts] = @SmServiceAccounts,
[MaxAutoscaleSmSeats] = @MaxAutoscaleSmSeats,
[MaxAutoscaleSmServiceAccounts] = @MaxAutoscaleSmServiceAccounts
WHERE
[Id] = @Id
END
5 changes: 5 additions & 0 deletions src/Sql/dbo/Tables/Organization.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@
[UseCustomPermissions] BIT NOT NULL CONSTRAINT [DF_Organization_UseCustomPermissions] DEFAULT (0),
[UseSecretsManager] BIT NOT NULL CONSTRAINT [DF_Organization_UseSecretsManager] DEFAULT (0),
[Status] TINYINT NOT NULL CONSTRAINT [DF_Organization_Status] DEFAULT (1),
[UsePasswordManager] BIT NOT NULL CONSTRAINT [DF_Organization_UsePasswordManager] DEFAULT (1),
cyprain-okeke marked this conversation as resolved.
Show resolved Hide resolved
[SmSeats] INT NULL,
[SmServiceAccounts] INT NULL,
[MaxAutoscaleSmSeats] INT NULL,
[MaxAutoscaleSmServiceAccounts] INT NULL,
CONSTRAINT [PK_Organization] PRIMARY KEY CLUSTERED ([Id] ASC)
);

Expand Down
5 changes: 4 additions & 1 deletion src/Sql/dbo/Views/OrganizationUserOrganizationDetailsView.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ SELECT
OS.[LastSyncDate] FamilySponsorshipLastSyncDate,
OS.[ToDelete] FamilySponsorshipToDelete,
OS.[ValidUntil] FamilySponsorshipValidUntil,
OU.[AccessSecretsManager]
OU.[AccessSecretsManager],
O.[UsePasswordManager],
O.[SmSeats],
O.[SmServiceAccounts]
cyprain-okeke marked this conversation as resolved.
Show resolved Hide resolved
FROM
[dbo].[OrganizationUser] OU
LEFT JOIN
Expand Down
Loading