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

REST API: Custom settings endpoint #1455

Closed
youknowriad opened this issue Jun 26, 2017 · 21 comments
Closed

REST API: Custom settings endpoint #1455

youknowriad opened this issue Jun 26, 2017 · 21 comments
Assignees
Labels
Core REST API Task Task for Core REST API efforts Framework Issues related to broader framework topics, especially as it relates to javascript [Status] Needs More Info Follow-up required in order to be actionable. [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@youknowriad
Copy link
Contributor

We need to persist different settings in different places of the Gutenberg Editor. Does it make sense to create a generic endpoint performing get/set_option?

We need it for:

cc @nylen

@youknowriad youknowriad added the [Type] Question Questions about the design or development of the editor. label Jun 26, 2017
@swissspidy
Copy link
Member

If it's just GET/POST for simple options, why not use the existing REST API endpoint for that?

@youknowriad
Copy link
Contributor Author

@swissspidy I was not aware of such endpoint, I must have missed it? Can't seem to find it in the REST API docs?

@swissspidy
Copy link
Member

@youknowriad here's the link to the docs: https://developer.wordpress.org/rest-api/reference/settings/. Registered settings can be fetched/saved using that endpoint.

@youknowriad
Copy link
Contributor Author

youknowriad commented Jun 26, 2017

Oh thanks for the clarification, I thought this endpoint would return only the settings mentioned in the docs.

So if I understand correctly, we'd need to register our settings using this right? https://codex.wordpress.org/Function_Reference/register_setting

The settings API seems more suited towards generating Admin Pages, is it ok to skip generating admin pages for those?

@swissspidy
Copy link
Member

Although the Settings API itself is meant for adding fields to admin pages (using add_settings_field() et al), register_setting() is specifically for adding and validating options and does not add anything to admin pages by itself. It's the recommended way to register settings for use with the REST API.

See https://make.wordpress.org/core/2016/10/26/registering-your-settings-in-wordpress-4-7/

Think of that function as a way to register a setting for the REST API, nothing else.

@youknowriad
Copy link
Contributor Author

youknowriad commented Jun 26, 2017

Awesome, thanks, this is really helpful. Will unblock some issues

@nylen
Copy link
Member

nylen commented Jun 29, 2017

We can't use wp/v2/settings for this because it requires admin privileges to both read and write settings.

@nylen nylen reopened this Jun 29, 2017
@youknowriad
Copy link
Contributor Author

And I think the settings we need to store should be per user. We need a user preferences endpoint.

@nylen
Copy link
Member

nylen commented Jun 29, 2017

Yes, good point. We could try a register_meta set up to integrate with the users endpoints. If permissions are an issue (we probably don't want this data to be publicly readable) then we can write a custom endpoint that still stores the data in a user meta value.

@mtias mtias added the Core REST API Task Task for Core REST API efforts label Jul 11, 2017
@notnownikki
Copy link
Member

We need an endpoint like this for #1859, did we come to a conclusion on this? I'm happy to start on a user preferences endpoint. (Because, I kinda need it to progress 😄 )

@nylen
Copy link
Member

nylen commented Jul 17, 2017

The conclusion is that we need a user preferences endpoint 😄 Feel free to give it a shot.

@westonruter
Copy link
Member

@nylen should we create a new gutenberg/v1 namespace to put all of the endpoints we need for developing blocks? This can be an interim location while we wait on them getting added to wp/v2 or whatever namespace we determine they'll live in, for example the logo block in #1851 could eventually use a logo endpoint under a customize/v1 namespace. It would seem like we should not get development on the blocks held up for lack of the requisite endpoints being in core.

@nylen
Copy link
Member

nylen commented Jul 17, 2017

Yes, our endpoints should live in gutenberg/v1 for now.

See also https://core.trac.wordpress.org/ticket/40919 which we can incorporate to provide a way to call the endpoints we need.

@nylen
Copy link
Member

nylen commented Jul 26, 2017

Removing "Core REST API Task" as this is something we can implement for ourselves for now.

@nylen nylen added Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Task Issues or PRs that have been broken down into an individual action to take and removed Core REST API Task Task for Core REST API efforts [Type] Question Questions about the design or development of the editor. labels Jul 26, 2017
@mtias mtias added the Core REST API Task Task for Core REST API efforts label Aug 18, 2017
@mtias
Copy link
Member

mtias commented Aug 18, 2017

The label is mostly meant to gather all the issues that need API work, regardless of where it is done.

@notnownikki
Copy link
Member

I reopened #1948 . There are discussions going on around userSettings which might give us what we need, but if not, that PR is there.

@BE-Webdesign
Copy link
Contributor

BE-Webdesign commented Sep 6, 2017

Do we 100% need to persist this to server? I think using the new storePersist middleware on our redux store would be fine. It would mean this wouldn't work across devices, but I don't think that would be the worst thing ever?

@notnownikki
Copy link
Member

I guess not 100%, but it would be nice if we were able to store data at a user level rather than a device+browser level.

@joehoyle
Copy link

The label is mostly meant to gather all the issues that need API work, regardless of where it is done.

It might make sense to rename the label, it's specifically "Core REST API" right now.

@danielbachhuber
Copy link
Member

Is this issue actionable at this point, or can it be closed?

@danielbachhuber danielbachhuber added the [Status] Needs More Info Follow-up required in order to be actionable. label Jan 23, 2018
@danielbachhuber
Copy link
Member

It looks like the first issue was solved with localStorage: #2568

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core REST API Task Task for Core REST API efforts Framework Issues related to broader framework topics, especially as it relates to javascript [Status] Needs More Info Follow-up required in order to be actionable. [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

No branches or pull requests

9 participants