Add support for optional Int id properties to Model
#104
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
djones6
suggested changes
Mar 5, 2019
README.md
Outdated
|
|
||
| ### Automatic id assignment | ||
|
|
||
| If your `Model` is defined without specifying an id field using the `idColumnName` property or the default name of `id` then the ORM will create an auto incrementing column named `id` in the database table for the `Model`, eg. |
There was a problem hiding this comment.
I find this sentence hard to parse. Could we add some punctuation?
README.md
Outdated
|
|
||
| ### Manual id assignment | ||
|
|
||
| You can add an id field to your model and manage the assignment of id’s yourself by either specifying a property name for your id field using the `idColumnName` property or by defining a field named `id`, which is the default name for the aforementioned property. For example: |
README.md
Outdated
|
|
||
| ### Using `optional` id fields | ||
|
|
||
| If you would like an id field that allows you to specify specific values whilst also being automatic when an id is not explicitly set you can use an optional Int for your id field and re-define the `idKeypath` property to point at the field. `IDKeyPath` is a typealias for `WritableKeyPath<Self, Int?>?`, your re-definition must be explicity set this type. For example: |
There was a problem hiding this comment.
Needs punctuation, and I think 'redefine' is not hyphenated. Also the second sentence doesn't look right - "your re-definition must be explicity set this type". Perhaps just "Your definition must be of type IDKeyPath, as in the example below". If someone really wants to know more about IDKeyPath they could look at the source.
README.md
Outdated
| } | ||
| ``` | ||
|
|
||
| **NOTE** - When using manual or option id fields you need to ensure that the application is written to handle violation of unique identifier constraints which will occur if you attempt to save a model with an id that already exists in the database. If you are using `SwiftKueryPostgreSQL` this error can occur when savinf with a `nil` value for the id property due to the way PostgreSQL implements `autoincrement` behaviour. |
There was a problem hiding this comment.
typo: saving
Also, if we're going to mention Postgres' behaviour specifically, we might want to elude to why (ie. that it does not skip over existing ids while incrementing).
djones6
approved these changes
Mar 19, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds support for optional Integer
idproperties to theModelprotocol.The README has been updated accordingly.