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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃棞 Set db soft limits #9225

Merged
merged 3 commits into from
Nov 9, 2017
Merged

馃棞 Set db soft limits #9225

merged 3 commits into from
Nov 9, 2017

Conversation

aileen
Copy link
Member

@aileen aileen commented Nov 8, 2017

refs #8143

Sets soft limits for certain db fields:

  • posts:

    • title: 255 chars (current hard limit: 2,000 chars)
    • meta_title: 300 chars (current hard limit: 2,000 chars)
    • meta_description: 500 chars (current hard limit: 2,000 chars)
  • users:

    • bio: 200 chars (current hard limit: 65,535 chars)
    • location: 150 chars (current hard limit: 65,535 chars)
    • meta_description: 500 chars (current hard limit: 2,000 chars)
    • meta_title: 300 chars (current hard limit: 2,000 chars)
  • tags:

    • description: 500 chars (current hard limit: 65,535 chars)
    • meta_title: 300 chars (current hard limit: 2,000 chars)
    • meta_description: 500 chars (current hard limit: 2,000 chars)
  • Added a specific error message in validate fn for isLength method.

  • validate will take the tableName value as optional argument to provide a user friendly error message when exceeding db soft limits

  • added tests for importer

refs TryGhost#8143

Sets soft limits for certain db fields:
- `posts`:
	- `title`: 255 chars (current hard limit: 2,000 chars)
	- `meta_title`: 300 chars (current hard limit: 2,000 chars)
	- `meta_description`: 500 chars (current hard limit: 2,000 chars)
- `users`:
	- `bio`: 200 chars (current hard limit: 65,535 chars)
	- `location`: 150 chars (current hard limit: 65,535 chars)
	- `meta_description`: 500 chars (current hard limit: 2,000 chars)
	- `meta_title`: 300 chars (current hard limit: 2,000 chars)
- `tags`:
	- `description`: 500 chars (current hard limit: 65,535 chars)
	- `meta_title`: 300 chars (current hard limit: 2,000 chars)
	- `meta_description`: 500 chars (current hard limit: 2,000 chars)
@aileen aileen force-pushed the soft-limits-db branch 3 times, most recently from 9b182cb to 8b29bd4 Compare November 9, 2017 05:04
- Added a specific error message in `validate` fn for `isLength` method.
- `validate` will take the `tableName` value as optional argument to provide a user friendly error message when exceeding db soft limits
- added more tests for importer
@@ -190,7 +190,12 @@ validateSchema = function validateSchema(tableName, model) {

// TODO: check if mandatory values should be enforced
if (model[columnKey] !== null && model[columnKey] !== undefined) {
// check length
// check soft limits first, if present (validations: {isLength: {min, max}})
if (_.has(schema[tableName][columnKey], 'validations.isLength')) {

This comment was marked as abuse.

This comment was marked as abuse.

@aileen
Copy link
Member Author

aileen commented Nov 9, 2017

Error message looks like this in Admin Client:

image

And this is the error itself:

NAME: ValidationError
MESSAGE: Value in [posts.meta_description] exceeds maximum length of 500 characters.

level:normal

posts.meta_description
ValidationError: Value in [posts.meta_description] exceeds maximum length of 500 characters.

Let me know, if this is an ok error message @kirrg001

- dynamic translation key reading
- you can define a custom translation for each validator
@kirrg001
Copy link
Contributor

kirrg001 commented Nov 9, 2017

Value in [posts.meta_description] exceeds maximum length of 500 characters.

I've double checked how the client shows the error.
It either shows Bio is too long or Meta Description cannot be longer than 500 characters..

It would be cool to be consistent here and show the same error message as the client.
But we can improve the error message as sub task of #8143 to get that in. The common case is to use the admin client and the error messages on the client look great. Could you please update the list? Thanks!

@kirrg001
Copy link
Contributor

kirrg001 commented Nov 9, 2017

@AileenCGN Don't care about the message improvement as part of #8143 , @ErisDS remembered me on this issue here.

@kirrg001 kirrg001 merged commit a35c0c2 into TryGhost:master Nov 9, 2017
@@ -80,6 +80,11 @@ I18n = {
return matchingString;
},

doesTranslationKeyExist: function doesTranslationKeyExist(msgPath) {

This comment was marked as abuse.

@aileen aileen deleted the soft-limits-db branch April 10, 2018 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants