develop -> staging

.github/workflows/build-and-run-tests.yml

@@ -3,9 +3,7 @@ name: Run automated tests
 on:
   pull_request:
     branches:
-      - develop
-      - staging
-      - main
+      - '**'
   push:
     branches:
       - develop
@@ -16,52 +14,52 @@ jobs:
   build-and-run-tests:
     runs-on: ubuntu-latest
     steps:
-    - name: Checkout code onto job runner
-      uses: actions/checkout@v2
+      - name: Checkout code onto job runner
+        uses: actions/checkout@v2
+
+      - name: Install .Net (6.0)
+        uses: actions/setup-dotnet@v1
+        with:
+          dotnet-version: 6.0.x
+
+      - name: Restore .Net dependencies
+        run: dotnet restore --configfile nuget.config
 
-    - name: Install .Net (6.0)
-      uses: actions/setup-dotnet@v1
-      with:
-        dotnet-version: 6.0.x
+      - name: Build .Net code
+        run: dotnet build --no-restore
 
-    - name: Restore .Net dependencies
-      run: dotnet restore --configfile nuget.config
+      - name: Run tests
+        run: dotnet test --no-restore --logger trx --results-directory "TestResults" --collect:"XPlat Code Coverage" --settings HerPortal.UnitTests/coverlet.runsettings
 
-    - name: Build .Net code
-      run: dotnet build --no-restore
+      - name: Inline code coverage report and minimum coverage check
+        uses: irongut/[email protected]
+        with:
+          filename: TestResults/*/coverage.cobertura.xml
+          badge: false
+          fail_below_min: true
+          format: text
+          hide_branch_rate: false
+          hide_complexity: false
+          indicators: true
+          output: console
+          # Threshold is low as this is a small project and a lot of the code is boilerplate or doesn't make sense to test
+          # Business logic coverage should be kept at/above 80%
+          thresholds: '50 80'
+        # Use always() to always run this step to publish coverage results when there are test failures
+        if: ${{ always() }}
 
-    - name: Run tests
-      run: dotnet test --no-restore --logger trx --results-directory "TestResults" --collect:"XPlat Code Coverage" --settings HerPortal.UnitTests/coverlet.runsettings
+      - name: Generate detailed code coverage report
+        uses: danielpalme/[email protected]
+        with:
+          reports: TestResults/*/coverage.cobertura.xml
+          targetdir: TestResults/CoverageReport
+        # Use always() to always run this step to publish coverage report when there are test failures
+        if: ${{ always() }}
 
-    - name: Inline code coverage report and minimum coverage check
-      uses: irongut/[email protected]
-      with:
-        filename: TestResults/*/coverage.cobertura.xml
-        badge: false
-        fail_below_min: true
-        format: text
-        hide_branch_rate: false
-        hide_complexity: false
-        indicators: true
-        output: console
-        # Threshold is low as this is a small project and a lot of the code is boilerplate or doesn't make sense to test
-        # Business logic coverage should be kept at/above 80%
-        thresholds: '50 80'
-      # Use always() to always run this step to publish coverage results when there are test failures
-      if: ${{ always() }}
-
-    - name: Generate detailed code coverage report
-      uses: danielpalme/[email protected]
-      with:
-        reports: TestResults/*/coverage.cobertura.xml
-        targetdir: TestResults/CoverageReport
-      # Use always() to always run this step to publish coverage report when there are test failures
-      if: ${{ always() }}
-
-    - name: Upload dotnet test results
-      uses: actions/upload-artifact@v3
-      with:
-        name: test-results
-        path: TestResults
-      # Use always() to always run this step to publish test results when there are test failures
-      if: ${{ always() }}
+      - name: Upload dotnet test results
+        uses: actions/upload-artifact@v3
+        with:
+          name: test-results
+          path: TestResults
+        # Use always() to always run this step to publish test results when there are test failures
+        if: ${{ always() }}

.github/workflows/main-label-check.yml

@@ -2,7 +2,13 @@ name: main_label_check
 
 on:
   pull_request:
-    types: [opened, edited, labeled, unlabeled]
+    types:
+      - opened
+      - edited
+      - labeled
+      - reopened
+      - synchronize
+      - unlabeled
 
 jobs:
   label_check:
@@ -13,5 +19,4 @@ jobs:
         uses: actions/github-script@v3
         with:
           script: |
-            core.setFailed("Label is missing: add a 'production release' label in order to merge to main")
-
+            core.setFailed("Label is missing: add a 'production release' label in order to merge to main") 

HerPortal.BusinessLogic/IDataAccessProvider.cs

@@ -9,5 +9,4 @@ public interface IDataAccessProvider
     public Task<IEnumerable<User>> GetAllActiveUsersAsync();
     public Task<List<CsvFileDownload>> GetCsvFileDownloadDataForUserAsync(int userId);
     public Task MarkCsvFileAsDownloadedAsync(string custodianCode, int year, int month, int userId);
-    public List<string> GetConsortiumCodesForUser(User user);
 }

HerPortal.BusinessLogic/Models/Consortium.cs

@@ -0,0 +1,15 @@
+namespace HerPortal.BusinessLogic.Models;
+
+/// <summary>
+///     This model is not a full model for Consortium Data.
+///     It should be considered as a reference to the full data for a Consortium.
+///     The full data (including an the Consortium's relationship to LAs) can be found in the HUG2 Public Website codebase
+///     <see href="https://github.com/UKGovernmentBEIS/desnz-home-energy-retrofit-beta/blob/develop/HerPublicWebsite.BusinessLogic/Models/LocalAuthorityData.cs"/>
+/// </summary>
+public class Consortium
+{
+    public int Id { get; set; }
+    public string ConsortiumCode { get; set; }
+
+    public List<User> Users { get; set; }
+}

HerPortal.BusinessLogic/Models/ConsortiumData.cs

@@ -1,7 +1,13 @@
 namespace HerPortal.BusinessLogic.Models;
 
+/// <summary>
+///     LA-Consortium mapping data can be found both in <see cref="LocalAuthorityData"/>, and in the HUG2 Public Website codebase's LocalAuthorityData.cs.
+///     <seealso href="https://github.com/UKGovernmentBEIS/desnz-home-energy-retrofit-beta/blob/develop/HerPublicWebsite.BusinessLogic/Models/LocalAuthorityData.cs">Link to HUG2 Public Website codebase's LocalAuthorityData.cs</seealso>
+///     Note: Mappings in the above files are expected to be consistent between both repos
+/// </summary>
 public static class ConsortiumData
 {
+    /// If any Custodian/Consortium codes are changed, consider if remapping is required in <see cref="LocalAuthorityData"/>
     public static readonly Dictionary<string, string> ConsortiumNamesByConsortiumCode = new()
     {
         { "C_0002", "Blackpool" },

HerPortal.BusinessLogic/Models/LocalAuthority.cs

@@ -1,5 +1,12 @@
 namespace HerPortal.BusinessLogic.Models;
 
+/// <summary>
+///     This model is not a full model for Local Authority Data.
+///     It should be considered as a reference to the full data for a Local Authority.
+///     Data for LA Relationships to Consortiums can be found in <see cref="LocalAuthorityData"> LocalAuthorityData.cs </see>
+///     The full data for LAs (including an LAs relationship to a Consortium) can also be found in the HUG2 Public Website codebase.
+///     <seealso href="https://github.com/UKGovernmentBEIS/desnz-home-energy-retrofit-beta/blob/develop/HerPublicWebsite.BusinessLogic/Models/LocalAuthorityData.cs">Link to HUG2 Public Website codebase's LocalAuthorityData.cs</seealso>
+/// </summary>
 public class LocalAuthority
 {
     public int Id { get; set; }

HerPortal.BusinessLogic/Models/LocalAuthorityData.cs

@@ -1,9 +1,20 @@
 namespace HerPortal.BusinessLogic.Models;
 
+/// <summary>
+///     This class does not contain all data for Local Authorities.
+///     Full data (including an LAs relationship to a Consortium) can be found in the HUG2 Public Website codebase
+///     <see href="https://github.com/UKGovernmentBEIS/desnz-home-energy-retrofit-beta/blob/develop/HerPublicWebsite.BusinessLogic/Models/LocalAuthorityData.cs"> HUG2 Public Website codebase </see>
+///     The LA-Consortium mapping contained here must be the same as the LocalAuthorityData.cs file in the
+///     public website repository.
+/// </summary>
 public static class LocalAuthorityData
 {
-    // The mapping from custodian code to name comes from the publicly available "Local custodian codes" download link
-    // on https://www.ordnancesurvey.co.uk/business-government/tools-support/addressbase-support
+    /// If Custodian Codes change (through LAs merging etc.) we need to ensure
+    /// consistency with the HUG2 Public Website data, and consider any remapping that may be required.
+    /// <see href="https://github.com/UKGovernmentBEIS/desnz-home-energy-retrofit-beta/blob/develop/HerPublicWebsite.BusinessLogic/Models/LocalAuthorityData.cs"> HUG2 Public Website codebase </see>
+    /// The mapping from custodian code to name comes from the publicly available "Local custodian codes" download link
+    /// on https://www.ordnancesurvey.co.uk/business-government/tools-support/addressbase-support
+    /// <see href=" https://www.ordnancesurvey.co.uk/business-government/tools-support/addressbase-support"/>
     public static readonly Dictionary<string, string> LocalAuthorityNamesByCustodianCode = new()
     {
         { "9052", "Aberdeenshire" },
@@ -376,6 +387,11 @@ public static class LocalAuthorityData
         { "2741", "York" },
     };
 
+    /// If Custodian Codes change (through LAs merging etc.) we need to ensure consistency with the
+    /// HUG2 Public Website data, and we may need to map old codes to new ones for existing users.
+    /// <see href="https://github.com/UKGovernmentBEIS/desnz-home-energy-retrofit-beta/blob/develop/HerPublicWebsite.BusinessLogic/Models/LocalAuthorityData.cs"> HUG2 Public Website codebase </see>
+    /// The mappings contained here should be consistent with the mapping in the HUG2 Public Website Repo's LocalAuthorityData.cs
+    /// <see href="https://github.com/UKGovernmentBEIS/desnz-home-energy-retrofit-beta/blob/develop/HerPublicWebsite.BusinessLogic/Models/LocalAuthorityData.cs"> HUG2 Public Website codebase </see>
     public static readonly Dictionary<string, string> LocalAuthorityConsortiumCodeByCustodianCode = new()
     {
         { "3805", "C_0031" },

HerPortal.BusinessLogic/Models/User.cs

@@ -5,6 +5,22 @@ public class User
     public int Id { get; set; }
     public string EmailAddress { get; set; }
     public bool HasLoggedIn { get; set; }
-    
+
     public List<LocalAuthority> LocalAuthorities { get; set; }
-}
+    public List<Consortium> Consortia { get; set; }
+
+    public IEnumerable<string> GetAdministeredCustodianCodes()
+    {
+        var consortiumCodes = Consortia.Select(consortium => consortium.ConsortiumCode);
+        var custodianCodes = LocalAuthorities.Select(la => la.CustodianCode);
+
+        return consortiumCodes.SelectMany(consortiumCode =>
+                ConsortiumData.ConsortiumCustodianCodesIdsByConsortiumCode[consortiumCode])
+            .Union(custodianCodes);
+    }
+
+    public IEnumerable<string> GetAdministeredConsortiumCodes()
+    {
+        return Consortia.Select(c => c.ConsortiumCode);
+    }
+}

HerPortal.BusinessLogic/Services/CsvFileService/CsvFileService.cs

@@ -83,17 +83,27 @@ public async Task<IEnumerable<CsvFileData>> GetFileDataForUserAsync(string userE
     {
         // Make sure that we only return file data for files that the user currently has access to
         var user = await dataAccessProvider.GetUserByEmailAsync(userEmailAddress);
-        var currentCustodianCodes = user.LocalAuthorities.Select(la => la.CustodianCode);
+        var custodianCodes = user.GetAdministeredCustodianCodes();
+        var consortiumCodes = user.GetAdministeredConsortiumCodes();
 
-        var downloads = await dataAccessProvider.GetCsvFileDownloadDataForUserAsync(user.Id);
-        var files = new List<CsvFileData>();
+        var localAuthoritiesFileData = await BuildCsvFileDataForLocalAuthorities(user, custodianCodes);
+        var consortiaTransformedFileData = TransformFileDataForConsortia(consortiumCodes, localAuthoritiesFileData);
+        var combinedFileData = localAuthoritiesFileData.Concat(consortiaTransformedFileData);
 
-        var consortiumCodes = dataAccessProvider.GetConsortiumCodesForUser(user);
+        return combinedFileData
+            .OrderByDescending(f => new DateOnly(f.Year, f.Month, 1))
+            .ThenByDescending(f => f is ConsortiumCsvFileData)
+            .ThenBy(f => f.Name);
+    }
 
+    private async Task<List<CsvFileData>> BuildCsvFileDataForLocalAuthorities(User user, IEnumerable<string> currentCustodianCodes)
+    {
+        var laFileData = new List<CsvFileData>();
+        var downloads = await dataAccessProvider.GetCsvFileDownloadDataForUserAsync(user.Id);
         foreach (var custodianCode in currentCustodianCodes)
         {
             var s3Objects = await s3FileReader.GetS3ObjectsByCustodianCodeAsync(custodianCode);
-            files.AddRange(s3Objects.Select(s3O =>
+            laFileData.AddRange(s3Objects.Select(s3O =>
                 {
                     var data = keyService.GetDataFromS3Key(s3O.Key);
                     var downloadData = downloads.SingleOrDefault(d =>
@@ -113,30 +123,28 @@ public async Task<IEnumerable<CsvFileData>> GetFileDataForUserAsync(string userE
             ));
         }
 
-        files.AddRange(
-            files
-                .Where(file => LocalAuthorityData.LocalAuthorityConsortiumCodeByCustodianCode.ContainsKey(file.Code))
-                .GroupBy(file => (LocalAuthorityData.LocalAuthorityConsortiumCodeByCustodianCode[file.Code], file.Month,
-                    file.Year))
-                .Where(grouping => consortiumCodes.Contains(grouping.Key.Item1))
-                .Select(grouping => new ConsortiumCsvFileData(
-                        grouping.Key.Item1, 
-                        grouping.Key.Month, 
-                        grouping.Key.Year, 
-                        grouping
-                            .Select(fileData => fileData.LastUpdated)
-                            .Max(), // there are csv updates as new as
-                        grouping
-                            .Select(fileData => fileData.LastDownloaded)
-                            .Min() // there are csvs undownloaded for as long as
-                    )
+        return laFileData;
+    }
+
+    private IEnumerable<CsvFileData> TransformFileDataForConsortia(IEnumerable<string> consortiumCodes, IEnumerable<CsvFileData> laFileData)
+    {
+        return laFileData
+            .Where(fileRow => LocalAuthorityData.LocalAuthorityConsortiumCodeByCustodianCode.ContainsKey(fileRow.Code))
+            .GroupBy(fileRow => (LocalAuthorityData.LocalAuthorityConsortiumCodeByCustodianCode[fileRow.Code], fileRow.Month,
+                fileRow.Year))
+            .Where(grouping => consortiumCodes.Contains(grouping.Key.Item1))
+            .Select(grouping => new ConsortiumCsvFileData(
+                    grouping.Key.Item1,
+                    grouping.Key.Month,
+                    grouping.Key.Year,
+                    grouping
+                        .Select(fileData => fileData.LastUpdated)
+                        .Max(), // there are csv updates as new as
+                    grouping
+                        .Select(fileData => fileData.LastDownloaded)
+                        .Min() // there are csvs undownloaded for as long as
                 )
             );
-
-        return files
-            .OrderByDescending(f => new DateOnly(f.Year, f.Month, 1))
-            .ThenByDescending(f => f is ConsortiumCsvFileData)
-            .ThenBy(f => f.Name);
     }
 
     // Page number starts at 1
@@ -167,7 +175,7 @@ public async Task<Stream> GetLocalAuthorityFileForDownloadAsync(string custodian
     {
         // Important! First ensure the logged-in user is allowed to access this data
         var userData = await dataAccessProvider.GetUserByEmailAsync(userEmailAddress);
-        if (!userData.LocalAuthorities.Any(la => la.CustodianCode == custodianCode))
+        if (!userData.GetAdministeredCustodianCodes().Contains(custodianCode))
         {
             // We don't want to log the User's email address for GDPR reasons, but the ID is fine.
             throw new SecurityException(
@@ -194,9 +202,8 @@ public async Task<Stream> GetConsortiumFileForDownloadAsync(string consortiumCod
     {
         // Important! First ensure the logged-in user is allowed to access this data
         var userData = await dataAccessProvider.GetUserByEmailAsync(userEmailAddress);
-        var consortiumCodes = dataAccessProvider.GetConsortiumCodesForUser(userData);
 
-        if (!consortiumCodes.Contains(consortiumCode))
+        if (!userData.GetAdministeredConsortiumCodes().Contains(consortiumCode))
         {
             // We don't want to log the User's email address for GDPR reasons, but the ID is fine.
             throw new SecurityException(

HerPortal.BusinessLogic/Services/UserService.cs

@@ -22,9 +22,4 @@ public async Task MarkUserAsHavingLoggedInAsync(int userId)
     {
         await dataAccessProvider.MarkUserAsHavingLoggedInAsync(userId);
     }
-
-    public List<string> GetConsortiumCodesForUser(User user)
-    {
-        return dataAccessProvider.GetConsortiumCodesForUser(user);
-    }
 }

HerPortal.Data/DataAccessProvider.cs

@@ -17,6 +17,7 @@ public async Task<User> GetUserByEmailAsync(string emailAddress)
     {
         var users = await context.Users
             .Include(u => u.LocalAuthorities)
+            .Include(u => u.Consortia)
             // In order to compare email addresses case-insensitively, we bring the whole table into memory here
             //   to perform the comparison in C#, since Entity Framework doesn't allow for the StringComparison
             //   overload. However, since we don't expect this table to be monstrously huge this is acceptable
@@ -44,7 +45,6 @@ public async Task<IEnumerable<User>> GetAllActiveUsersAsync()
     {
         return await context.Users
             .Where(u => u.HasLoggedIn)
-            .Include(u => u.LocalAuthorities)
             .ToListAsync();
     }
 
@@ -102,15 +102,4 @@ public async Task MarkCsvFileAsDownloadedAsync(string custodianCode, int year, i
 
         await context.SaveChangesAsync();
     }
-
-    public List<string> GetConsortiumCodesForUser(User user)
-    {
-        var userLocalAuthorities = user.LocalAuthorities.Select(la => la.CustodianCode);
-
-        // user is a consortium manager if they are a manager of all LAs in that consortium
-        return ConsortiumData.ConsortiumCustodianCodesIdsByConsortiumCode
-            .Where(pair => pair.Value.All(consortiumLa => userLocalAuthorities.Contains(consortiumLa)))
-            .Select(pair => pair.Key)
-            .ToList();
-    }
 }

HerPortal.Data/HerDbContext.cs

@@ -11,6 +11,7 @@ public class HerDbContext : DbContext, IDataProtectionKeyContext
     public DbSet<AuditDownload> AuditDownloads { get; set; }
     public DbSet<CsvFileDownload> CsvFileDownloads { get; set; }
     public DbSet<LocalAuthority> LocalAuthorities { get; set; }
+    public DbSet<Consortium> Consortia { get; set; }
     public DbSet<User> Users { get; set; }
 
     public HerDbContext(DbContextOptions<HerDbContext> options) : base(options)