EES-4947 - updating version number after saves #5102
Conversation
duncan-at-hiveit
changed the title
| /// modelBuilder.Entity<JsonInt>().HasNoKey().ToView(null); | ||
| /// </code> | ||
| /// </summary> | ||
| public record JsonInt(int IntValue); |
There was a problem hiding this comment.
I don't think this is currently used. Are we just putting it here in case we need it in the future (which is absolutely fine)?
There was a problem hiding this comment.
Yup that's it! I was using it to capture a SELECT 1 FROM for an existence check, but then in order to work out the major / minor version numbers, I needed actual MappingType enum values back, so I swapped it out, but decided to keep it around, as you rightly guessed!
| public DbSet<JsonFragment> JsonFragments { get; init; } = null!; | ||
|
|
||
| public DbSet<JsonBool> JsonBool { get; init; } = null!; |
There was a problem hiding this comment.
I'm guessing these aren't actually needed?
There was a problem hiding this comment.
Yeah, you can use them to do context.JsonFragments.Where...-stle queries, but equally you can use context.Set<JsonFragments>().Where... instead. Given that a lot of the PSQL code lives in a common codebase that takes a generic DbContext argument, we use the latter form there, so it felt better to use it elsewhere as well.
| @@ -20,7 +20,12 @@ internal class DataSetVersionMappingService( | |||
| { | |||
| private static readonly MappingType[] IncompleteMappingTypes = | |||
| [ | |||
| MappingType.None, | |||
| MappingType.AutoNone | |||
There was a problem hiding this comment.
Are we omitting None from here because theoretically, at this stage of the code, it should never be in this stage?
There was a problem hiding this comment.
Yup exactly!
| @@ -370,6 +371,242 @@ originalCountryMappingToUpdate with | |||
| updatedMappings.LocationMappingPlan.Levels.AssertDeepEqualTo( | |||
| expectedFullMappings, | |||
| ignoreCollectionOrders: true); | |||
|
|
|||
| // Assert that the batch saves still show the location mappings as incomplete, as there | |||
| // are still mappings with type "None" and "AutoNone" in the plan. | |||
There was a problem hiding this comment.
should this comment read ... are still mappings with type "AutoNone" in the plan. I think None won't be present will it?
There was a problem hiding this comment.
Updated ta, plus updated the initial test setup to be more realistic
duncan-at-hiveit
force-pushed
the
EES-4947-calculate-version-number-on-automap-and-autosave-3
branch
from
e04852f
to
0c8b418
Compare
…ial test state to reflect more realistic setup.
duncan-at-hiveit
force-pushed
the
EES-4947-calculate-version-number-on-automap-and-autosave-3
branch
from
0c8b418
to
1161854
Compare
duncan-at-hiveit
deleted the
EES-4947-calculate-version-number-on-automap-and-autosave-3
branch
Overview
This PR:
The main idea behind this PR is that we update the expected version number of the next data set version whenever we can, so we can always see what the next version number should be based upon the mapping work undertaken so far.
The other secondary part of this PR ensures that our "MappingsComplete" flags are always up-to-date after autosaves have completed.
Creating a better initial version number for the next data set version
When we very first create the skeleton entry for the next DataSetVersion, we allocate it a minor version number update from its predecessor, as we must give it something and it must be unique.
After auto-mappings have completed however, we already can have a really good idea as to whether this should be a minor or major update. If for instance an entire geographic level has been removed from the next data set version or an entire filter has been removed, or if the number of locations or filters / filter options are less than the original data set version, we can be assured that the next data set version's version number jump will be a major one.
This PR therefore accommodates this and allocates a realistic version number as early as possible.
Updating the "MappingsComplete" flags after autosave
This wasn't strictly part of this ticket to do, but it has to be done somewhere and it was somewhat related code-wise with the changes being made for calculating version numbers.
After autosaves complete, we're able to determine if the mappings for a given facet are complete based upon the distinct MappingTypes present within the various mappings of that facet. "LocationMappingsComplete" can be determined for instance by finding all of the distinct MappingTypes across all of the geographic levels, and seeing if any exist that are "AutoNone".
Similarly, "FilterMappingsComplete" can be determined for instance by finding all of the distinct filter option MappingTypes across all of the filters, and seeing if any exist that are "AutoNone". The important distinction here as opposed to locations however is that if a filter itself has not been successfully auto-mapped, we exclude its filter options from this calculation, because for MVP we have no way to resolve unmapped filters and so we would otherwise end up in a situation where people couldn't complete their filter mappings. This is why we need to store filter and option mapping type combinations, and hence why we need to register a DTO specifically for the purpose of pulling these mapping type pairs out of the JSONB query results.
Updating the projected version number after autosave
To calculate the most likely next version number after an autosave occurs, we need to detect any breaking changes and, if any exist, we make it a major update as opposed to a minor one.
To do this, we take the same information about distinct Mapping Types as we used for updating the "MappingsComplete" flags and use it to determine if there are any mapping types that constitute a breaking change. In other words, we look to see if there are any mappings with type "AutoNone" or "ManualNone", as they indicate that thus far, there is at least one source element that has not successfully been mapped to a candidate. If we find any, this constitutes a major version update and so we set that on the next data set version. Otherwise we set a minor version update on it.