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

Filters out private taxonomies from sidebar UI. #2654

Merged
merged 8 commits into from Apr 13, 2018

Conversation

Projects
None yet
7 participants
@BE-Webdesign
Contributor

BE-Webdesign commented Sep 4, 2017

Fixes #2450,

Could potentially break if the user does not have privileges to see the
edit context. Will need to test more.

Testing Instructions

  1. Register a private taxonomy.
  2. Load Gutenberg.
  3. Verify that the taxonomy does not show up in the Categories & Tags portion of the sidebar.
@codecov

This comment has been minimized.

codecov bot commented Sep 4, 2017

Codecov Report

Merging #2654 into master will decrease coverage by 6.19%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2654     +/-   ##
=========================================
- Coverage   39.61%   33.42%   -6.2%     
=========================================
  Files         356      188    -168     
  Lines        8587     5639   -2948     
  Branches     1480      982    -498     
=========================================
- Hits         3402     1885   -1517     
+ Misses       4446     3180   -1266     
+ Partials      739      574    -165
Impacted Files Coverage Δ
editor/sidebar/post-taxonomies/index.js 0% <ø> (ø)
components/panel/row.js 0% <0%> (-100%) ⬇️
blocks/url-input/button.js 0% <0%> (-100%) ⬇️
blocks/block-controls/index.js 0% <0%> (-100%) ⬇️
blocks/color-palette/index.js 0% <0%> (-90%) ⬇️
components/keyboard-shortcuts/index.js 0% <0%> (-85.72%) ⬇️
blocks/alignment-toolbar/index.js 16.66% <0%> (-83.34%) ⬇️
blocks/block-alignment-toolbar/index.js 33.33% <0%> (-55.56%) ⬇️
components/spinner/index.js 50% <0%> (-50%) ⬇️
components/drop-zone/index.js 5.88% <0%> (-49.12%) ⬇️
... and 432 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec8fcba...c6bd6dc. Read the comment docs.

@aduth

Works well in my testing.

@@ -0,0 +1,44 @@
<?php
/**
* Internationalization-related functions for the Gutenberg editor plugin.

This comment has been minimized.

@aduth

aduth Sep 6, 2017

Member

Inaccurate description. Probably something about these being enhancements to existing endpoints? A bit unclear where the line should be drawn between compat.php and this one.

This comment has been minimized.

@BE-Webdesign

BE-Webdesign Sep 6, 2017

Contributor

Lol woops, copy pasta.

This comment has been minimized.

@BE-Webdesign

BE-Webdesign Sep 6, 2017

Contributor

This should probably go into compat.php. Didn't realize what that was until now.

@@ -35,9 +35,11 @@ class PostTaxonomies extends Component {
componentDidMount() {
this.fetchTaxonomies = new wp.api.collections.Taxonomies()
.fetch()

This comment has been minimized.

@aduth

aduth Sep 6, 2017

Member

Maybe not here, but we should see about upgrading this component to using the withAPIData higher-order component.

https://github.com/WordPress/gutenberg/tree/master/components/higher-order/with-api-data

This comment has been minimized.

@youknowriad

youknowriad Sep 6, 2017

Contributor

already done and merged, needs rebase :)

This comment has been minimized.

@BE-Webdesign

BE-Webdesign Sep 6, 2017

Contributor

I will rebase and use the component.

.done( ( taxonomies ) => {
this.setState( { taxonomies: Object.values( taxonomies ) } );
this.setState(
{ taxonomies: Object.values( taxonomies ).filter( taxonomy => taxonomy.public ) }

This comment has been minimized.

@aduth

aduth Sep 6, 2017

Member

Minor: Lodash's filter handles this beautifully (coercing object to array, filtering on key):

this.setState( {
	taxonomies: filter( taxonomies, { public: true } ),
} );

This comment has been minimized.

@BE-Webdesign

BE-Webdesign Sep 6, 2017

Contributor

Was thinking about using Lodash, this looks perfect!

@BE-Webdesign

This comment has been minimized.

Contributor

BE-Webdesign commented Sep 24, 2017

@aduth, @youknowriad. This should be ready now. Rebased to use the new method Riad implemented, using higher order component. Does not fully work for authors, but that is tied to a larger problem.

@BE-Webdesign

This comment has been minimized.

Contributor

BE-Webdesign commented Sep 24, 2017

Fixes #2450,

Adds a public property to the taxonomy field, this is used to determine
whether the taxonomy is public or private. Private taxonomies will now
properly be hidden. Does not work 100% properly for Authors, this is a larger problem with
REST API.

@BE-Webdesign

This comment has been minimized.

Contributor

BE-Webdesign commented Sep 25, 2017

Anybody know why the test failed?

Fatal error: Class 'WP_Block_Type' not found in /home/travis/build/WordPress/gutenberg/phpunit/class-wp-dummy-block-type.php on line 11

I don't think that has anything to do with this PR. Maybe re start the build?

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented Sep 25, 2017

@BE-Webdesign you should try to rebase, this probably means the build will fail on master if you merge, due to some other PR.

@BE-Webdesign

This comment has been minimized.

Contributor

BE-Webdesign commented Sep 25, 2017

Pretty sure I rebased against master. I will rebase again.

@BE-Webdesign

This comment has been minimized.

Contributor

BE-Webdesign commented Sep 25, 2017

That seemed to fix it, but I dunno why, it didn't bring in any new files etc.

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented Sep 25, 2017

computers 💻 are weird :)

@BE-Webdesign

This comment has been minimized.

Contributor

BE-Webdesign commented Sep 25, 2017

computers 💻 are weird :)

Yup, this happens all too often 😅 .

@youknowriad youknowriad requested a review from rmccue Sep 25, 2017

@youknowriad

Works great! I wonder if this fields should not be added by default to the Core Endpoint.

Does not work 100% properly for Authors, this is a larger problem with REST API.

Could you detail the issue here and maybe create an issue/ticket for it?

Since we decided to start the API work here in Gutenberg and potentially move them to Core later, this looks good to me.

@BE-Webdesign

This comment has been minimized.

Contributor

BE-Webdesign commented Sep 25, 2017

Could you detail the issue here and maybe create an issue/ticket for it?

#2545 is the ticket explaining most of it. Basically authors cannot view taxonomy data ever, due to permissions.

This PR, should magically work for authors, once #2545 is resolved, but they are somewhat separate issues.

@BE-Webdesign

This comment has been minimized.

Contributor

BE-Webdesign commented Sep 25, 2017

I wonder if this fields should not be added by default to the Core Endpoint.

It is on the edit context, which is a restricted context, so this will only be visible to authenticated users. I am not sure what other conditional we would want to add to hide it. I think it should be fine as is. But I will wait for @rmccue review, before merging.

@BE-Webdesign

This comment has been minimized.

Contributor

BE-Webdesign commented Sep 29, 2017

I think this problem is actually slightly more sinister than previously thought. I have to verify, but I think private taxonomies will still display on a post page as long as they have a valid metabox callback set. So maybe we need to use that instead.

@pento

This comment has been minimized.

Member

pento commented Nov 27, 2017

I've created a core ticket to expose the visibility settings for taxonomies.

Note: To mirror Core behaviour, you need to test the show_ui setting, not the public setting.

@pento

This comment has been minimized.

Member

pento commented Feb 22, 2018

The Core ticket has just landed in trunk. @BE-Webdesign, do you have bandwidth to update this PR?

@schlessera

This comment has been minimized.

Member

schlessera commented Mar 6, 2018

I did the following changes to update this PR:

  • rebased to latest master
  • added polyfill to backport https://core.trac.wordpress.org/ticket/42707
  • adapted unit tests to check for visibility properties
  • adapted post taxonomy component to hide on visibility.show_ui: false
  • added test to ensure this works as expected

BE-Webdesign and others added some commits Sep 4, 2017

Filters out private taxonomies from sidebar UI.
Fixes #2450,

Adds a public property to the taxonomy field, this is used to determine
whether the taxonomy is public or private. Private taxonomies will now
properly be hidden. Does not work 100% properly for Authors, this is a larger problem with
REST API.
Add visibility property array
This is a polyfill for Core #42707 so that Gutenberg can be tested on older versions of Core.

See https://core.trac.wordpress.org/ticket/42707

@danielbachhuber danielbachhuber added this to the 2.7 milestone Apr 13, 2018

@danielbachhuber

This comment has been minimized.

Member

danielbachhuber commented Apr 13, 2018

I rebased this against master and tested locally with the following:

add_action( 'init', function() {
    register_taxonomy( 'secrets', array( 'post' ), array(
        'label'             => 'Secrets',
        'public'            => true,
        'show_ui'           => false,
        'show_in_rest'      => true,
        'rest_base'         => 'secrets',
    ) );
});

Using this pull request, the "Secrets" tag UI isn't displayed as expected.

@danielbachhuber danielbachhuber merged commit 0c5aa0d into master Apr 13, 2018

2 checks passed

codecov/project 44.45% (+<.01%) compared to c199a4b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@danielbachhuber danielbachhuber deleted the fix/hide-private-taxonomies branch Apr 13, 2018

@mtias

This comment has been minimized.

Contributor

mtias commented Apr 18, 2018

Nice to see this in!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment