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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subscribers cannot be created manually #10569

Closed
kevinansfield opened this issue Mar 5, 2019 · 5 comments
Closed

Subscribers cannot be created manually #10569

kevinansfield opened this issue Mar 5, 2019 · 5 comments
Labels
bug [triage] something behaving unexpectedly help wanted [triage] Ideal issues for contributors to help with server / core Issues relating to the server or core of Ghost stale [triage] Issues that were closed to to lack of traction

Comments

@kevinansfield
Copy link
Contributor

Issue Summary

When creating a subscriber manually via the admin interface you get an SQL error.

ERROR [2019-03-05 16:23:07] "POST /ghost/api/v2/admin/subscribers/" 500 38ms

NAME: InternalServerError
CODE: ER_BAD_NULL_ERROR
MESSAGE: insert into `subscribers` (`created_at`, `created_by`, `email`, `id`, `name`, `post_id`, `status`, `subscribed_referrer`, `subscribed_url`, `unsubscribed_at`, `unsubscribed_url`, `updated_at`, `updated_by`) values ('2019-03-05 16:23:07', '5c7ea26b419824ea80001aa2', 'test@example.com', '5c7ea26b419824ea80001aa2', NULL, NULL, NULL, NULL, NULL, NULL, NULL, '2019-03-05 16:23:07', '5c7ea26b419824ea80001aa2') - ER_BAD_NULL_ERROR: Column 'status' cannot be null

To Reproduce

  1. Open the admin area
  2. Enable subscribers under labs if it's not already enabled
  3. Open the subscribers screen
  4. Try adding a subscriber using the Add Subscriber button

Technical details:

  • Ghost Version: 2.16.3
  • Node Version: 10.15.1
  • Browser/OS: Chrome/macOS
  • Database: MySQL
@kevinansfield kevinansfield added bug [triage] something behaving unexpectedly server / core Issues relating to the server or core of Ghost labels Mar 5, 2019
@kirrg001 kirrg001 added the affects:admin Anything relating to Ghost Admin label Mar 5, 2019
@kirrg001 kirrg001 removed the affects:admin Anything relating to Ghost Admin label Mar 5, 2019
@kirrg001
Copy link
Contributor

kirrg001 commented Mar 5, 2019

refs 449bae9


The admin client sends {status: null}, which gets forwarded to the database and throws an error because the field is not nullable. As soon as a field is set to null explicitly, the default values don't kick in (not on db layer, nor on model layer). The default value for "status" is "subscribed". It is only set if you don't send the "status" field at all.

We could allow passing null values if the field has a default value and remove these fields in API layer.

But currently, the e.g. post/page JSON validation schema does not allow sending visibility:null. It will give you a validation error, because "null" is not a valid value. And "visibility" has a default value too.

We either need to allow this behaviour for all resources (remove fields which are null and have a default value) or we have to throw a validation error.

kevinansfield added a commit to TryGhost/Admin that referenced this issue Mar 6, 2019
refs TryGhost/Ghost#10569
- updates the Subscriber serialiser to strip the `status` property from the API request when saving if it's falsy
@kirrg001
Copy link
Contributor

kirrg001 commented Mar 6, 2019

Short-term fix was added via TryGhost/Admin@996cc16.

@kirrg001 kirrg001 removed their assignment Mar 6, 2019
@allouis
Copy link
Contributor

allouis commented Mar 26, 2019

This issue is fixed now - if there is still outstanding work for allowing null values to be sent to the API, should we track it under a more specific issue? @kirrg001

@kirrg001
Copy link
Contributor

if there is still outstanding work

Yes the issue was not solved on the server side.

should we track it under a more specific issue

Can do that or rename/extend the issue description 👍

@ErisDS ErisDS added the help wanted [triage] Ideal issues for contributors to help with label Mar 28, 2019
@stale
Copy link

stale bot commented Jun 26, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale [triage] Issues that were closed to to lack of traction label Jun 26, 2019
@stale stale bot closed this as completed Jul 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [triage] something behaving unexpectedly help wanted [triage] Ideal issues for contributors to help with server / core Issues relating to the server or core of Ghost stale [triage] Issues that were closed to to lack of traction
Projects
None yet
Development

No branches or pull requests

4 participants