feat: Remove collection from patch schema #1957
Conversation
AndrewSisley
added
feature
AndrewSisley
changed the title
AndrewSisley
force-pushed
the
1956-rm-col-from-patch-schema
branch
from
8499f0d
to
6abf0c4
Compare
Codecov ReportAll modified lines are covered by tests ✅
@@ Coverage Diff @@
## develop #1957 +/- ##
===========================================
- Coverage 74.48% 74.25% -0.23%
===========================================
Files 242 242
Lines 23479 23436 -43
===========================================
- Hits 17486 17401 -85
- Misses 4812 4847 +35
- Partials 1181 1188 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 9 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
db/schema.go
Outdated
| fieldIndexesByName := make(map[string]int, len(col.Schema.Fields)) | ||
| fieldIndexesByCollection[colName] = fieldIndexesByName | ||
| for i, field := range col.Schema.Fields { | ||
| fieldIndexesByCollection := make(map[string]map[string]int, len(schemaByName)) |
There was a problem hiding this comment.
suggestion: fieldIndexesBySchema
There was a problem hiding this comment.
cheers, changing :)
- Rename var to fieldIndexesBySchema
db/collection.go
Outdated
| @@ -228,12 +206,19 @@ func (db *db) createCollection( | |||
| func (db *db) updateCollection( | |||
There was a problem hiding this comment.
todo: Spotted by Fred - this should no longer be called updateCollection.
- Rename to
updateSchema
There was a problem hiding this comment.
Nothing that it should also be moved to schema.go
There was a problem hiding this comment.
I think it should be co-located with the other functions that update the descriptions (including the future patchCollection), spreading those functions out will not aid readability.
Your thought should be re-visited in #1963 once everything in this space makes a little more sense and we can move everything together.
db/schema.go
Outdated
| for _, schema := range newSchemaByName { | ||
| if schema.Name == "" { | ||
| return ErrSchemaNameEmpty | ||
| } | ||
|
|
||
| collectionDescription, ok := collectionsByName[schema.Name] | ||
| if !ok { | ||
| return NewErrAddCollectionWithPatch(schema.Name) | ||
| } | ||
|
|
||
| col, err := db.newCollection(collectionDescription, schema) |
There was a problem hiding this comment.
thought: If we change the current updateCollection to updateSchema, we will still need an updateCollection function to handle the collection specific update to the new schema version. This part might be better off in that new updateCollection function.
AndrewSisley
force-pushed
the
1956-rm-col-from-patch-schema
branch
from
6abf0c4
to
6198e35
Compare
AndrewSisley
force-pushed
the
1956-rm-col-from-patch-schema
branch
from
b8ea345
to
fe8bde0
Compare
## Relevant issue(s) Resolves sourcenetwork#1956 ## Description Removes collectionDescription from patch schema. PatchSchema now only uses collectionDescription internally to update it if requested (bool param). This now means that the only place remaining (minus a util func that got missed) that refs collectionDescription.Schema should be the serialization. This will be sorted out in a later PR, before removal of the property completely.
## Relevant issue(s) Resolves sourcenetwork#1956 ## Description Removes collectionDescription from patch schema. PatchSchema now only uses collectionDescription internally to update it if requested (bool param). This now means that the only place remaining (minus a util func that got missed) that refs collectionDescription.Schema should be the serialization. This will be sorted out in a later PR, before removal of the property completely.
Relevant issue(s)
Resolves #1956
Description
Removes collectionDescription from patch schema.
PatchSchema now only uses collectionDescription internally to update it if requested (bool param).
This now means that the only place remaining (minus a util func that got missed) that refs collectionDescription.Schema should be the serialization. This will be sorted out in a later PR, before removal of the property completely.