humanmade / authorship Goto Github PK
View Code? Open in Web Editor NEWA modern approach to author attribution in WordPress.
License: GNU General Public License v3.0
A modern approach to author attribution in WordPress.
License: GNU General Public License v3.0
The default "Author" control should be removed from the block editor screen. Note that in WordPress 5.6 this will change from a standard <select>
to a searchable input, so the removal needs to work in 5.6 as well as earlier versions.
I did some cursory research and it seems there's no means of removing this programatically unless support for author
is removed from the post type, which isn't desirable.
Other things to try:
A user with a role of Guest Author should not be able to log in at all.
wp-login.php
The Disable Accounts plugin should do most of what we need. If there's anything missing, let's send fixes upstream to that plugin too.
Implement a basic template function that accepts a post object and returns array of WP_User
objects related to the post. The taxonomy term that creates the connection between post and users is abstracted from and never seen by the user.
Unfortunately we weren't able to use Reach UI Combobox as it doesn't support multiple selection.
Some cursory Googling tells us that React Select by default has some accessibility issues. These need to be tested, listed, and then investigated.
When updating posts via WP-CLI with wp post update
, there should be a flag that can be used to set multiple post authors.
--authorship=1,2,3
wp_insert_post()
, but needs investigatingNot so useful at the moment but will be useful in the context of Full Site Editing.
Ref
authorship/tests/phpunit/test-capabilities.php
Lines 330 to 332 in 7fa4c1c
Any considerations? Might need to check how popular SEO plugins work and whether we need to write any integration.
As already discussed, I have taken some time to review the current state of Authorship, looking both at the codebase, the functionality and user experience, and developer tooling.
Here's what I've got!
Some of the things I already addressed in a PR. Feel free to take as is, or cherry-pick stuff you like, or close entirely. I thought it made sense to implement some of the changes that are quick for me to do, and just a yes/no decision for you. There's more to do where I didn't want to pose anything, so I left these things completely for you.
The input element is, by default, just as wide as the content. If it's empty, this means that this is just about 2 pixels, making the cursor not appear as Text tool:
I suggest to always render the input as wide as its container element:
I know that you already have some further UX improvements in mind, including label, border etc., so I'm not going to comment on any of that.
In my opinion, both the index.tsx
and the authors-select.tsx
files are doing too much (different things, even).
In the end, the index file will register a plugin that renders the Authorship multi-select input into the PluginPostStatusInfo
slot, located in Status & visibility panel. The settings and code that make up that plugin/select would ideally live in one or more dedicated files.
The authors-select.tsx
file includes type definitions/interfaces, helper functions, and the actual component, but not the HOC definitions from the index.tsx
file. I suggest to split off all type definitions into a new types.ts
file, move the helper function into a new utils.ts
file or better even individually into utils/arrayMove.ts
, and move the complete definition of the Select
component, which is currently located in the index file, into the component file. Here, I would extract the anonymous arrow functions passed to both withDispatch
and withSelect
to allow for better readability and testability (if JS unit tests were ever to get added).
Inside the authors-select.tsx
file, both the createOption
and the formatOptionLabel
functions are independent of anything defined in the AuthorsSelect
component/function, hence I suggest to move these functions out of the component. There's no point in redeclaring them on every render.
The SortableMultiValueElement
component is independent of anything other than react-select
and react-sortable-hoc
, meaning only third-party packages. I suggest to move this into a new component file.
And the SortableSelectContainer
, as well as the Select
component could live in a separate file, too. This would allow for abstracting away some hard-coded props such as class names, formatting, and further configuration, and it would also move the new SortableMultiValueElement
component out of the authors-select.tsx
file.
Based on previous projects, I suggest the following architecture and file organization, allowing for a better separation of concerns, and thus easier maintainability:
src
+- components
| +- AuthorsSelect.tsx
| +- SortableMultiValueElement.tsx
| +- SortableSelectContainer.tsx
+- utils
| +- arrayMove.ts
+- index.tsx
+- plugin.tsx
+- style.scss
+- types.ts
π I addressed all of the above in #42.
Why did you decide to use inline styles in the formatOptionLabel
function? Could we just add a custom CSS class instead, and define the styles in the style.scss
file?
Also, do we really need the additional wrapper around the avatar?
wp
GlobalI suggest to not directly reference the wp
global, but instead import anything WordPress off the respective @wordpress/...
packages/externals.
The two wp.apiFetch
references would then be replaced by import apiFetch from '@wordpress/api-fetch';
, and wp.plugins.registerPlugin
would become import { registerPlugin } from '@wordpress/plugins';
. For the latter, we would need to add the @types/wordpress__plugins
dependency, as TypeScript doesn't recognize the registerPlugin
, for whatever reason. And due to a minor bug in the typings, we now have to pass the icon
property in the settings
object. It should be marked as optional, but it's not. Adding icon: null
is not a big deal, though.
π I addressed this in #42.
This also means that we can remove the declare const wp: any;
declarations, since wp
is not used anymore.
PhpDoc is "aware" of the current file's scope and imports. So, if you imported (i.e., use
d) some class, you don't have to provide its FQN. Therefore, you can remove the leading backslash for all WP class such as Β΄\WP` in parameter or return types.
π I addressed this in #40.
In the namespace.php
file, there are a few places where you "remove" from a given list of capabilities all values that you defined elsewhere. You are doing this by using array_filter
and in_array
:
$caps = array_filter( $caps, function( string $cap ) use ( $remove ) : bool {
return ! in_array( $cap, $remove['delete'], true );
} );
Is there any reason you are not using array_diff
?
$caps = array_diff( $caps, $remove['delete'] );
This is much less code, no extra function, and should result in the same list of capabilities...?
π I addressed this in #40.
Looking at the code, it seems you defined the singatures of action and filter callbacks to be all arguments provided by do_action
and apply_filters
, respectively.
Personally, I feel this is a bit of unuseful information, especially if you have @param
documentation, too. I understand this will save yourself some time if you ever were to need one or more of the currently unused arguments, but it might as wellmean more time spent when creating the function initially. But this is personal preference, I guess. π
Instead of accessing globals directly, I suggest to make use of API functions, where available.
For example, I would use is_author()
instead of $wp_query->is_author()
(where you first have to "import" the $wp_query
global anyway), and get_query_var()
instead of $wp_query->get()
. This also makes for easier (proper) unit testing.
π I addressed this in #40.
Instead of checking $unsanitized_postarr['tax_input'][ TAXONOMY ]
inside the filter callback added to wp_insert_post
, I suggest to check it beforehand, and only register the callback if necessary. Or do you expect it to change (which is technically possible, of course, but not what should happen)...?
In the nested filter callback added to views_edit-{$post_type}
in the init_taxonomy
function, there is some data that is independent of the individual post type, and yet it is computed over and over again. I suggest to move this out to happen before the array_map
(orrather: array_walk
, see further down) call. Things I am looking at include the user ID, the term object, and maybe even parts of the "mine" link/message.
π I addressed this in #40.
Still in the same nested filter callback, if there is no all
view (because a plugin removed it), there will also be no mine
view as it depends on all
to exist. Instead of the more manual foreach
-and-compare approach, I would not unset the "mine" filter always, but overwrite it with the new link. And only if there is no term object for the current user, which means there is no post where they are any kind of author, I would remove the "mine" link.
π I addressed this in #40.
In the filter_rest_post_dispatch
function in namespace.php
, if the author param is set, but not an array, the foreach
will break. I'm not sure what the best way to handle this is, but you could either check for array, or cast to array, or something else entirely.
In the init_taxonomy
function in namespace.php
, you are using array_map
, but you are not doing anything with the data. I assume this should be array_walk
, or simply foreach
instead?
π I addressed this in #40.
Same thing in the init_admin_cols
function in the admin.php
file. I feel this should be a foreach
, not requiring a new anonymous function, but could be array_walk
as well. Just not array_map
. π
π I addressed this in #40.
Similar to the "mine" view mentioned before, the filter_post_columns
function in admin.php
, will not add the authorship admin column if the native "author" column has been removed (by some other plugin). If this is what you want, then cool. If we want to render the authorship column always, and just make sure the author one does not render, you would need to adapt the code.
I feel the 'https://authorship.hmn.md/{rel}'
string, used in filter_rest_response_link_curies
could be defined as a constant, just like all the other bits and pieces.
π I addressed this in #40.
The namespace.php
file is quite long. How do you feel about splitting of everything directly related to the authorship
taxonomy into a new, taxonomy.php
file?
Do you think its necessary to preload the author data?
I mean, in theory it's not a bad idea, of course. But there are a few advantages to making an explicit REST API request from within the Block Editor to fetch the author data. The native author select is super slow and I can't think of a reason why users would need to see the UI instantly.
If we'd request the data from the JavaScript app, we wouldn't need to rely on a global variable, which means less manual TypeScript stuff.
But maybe this is what you mean with the TODO not in that function. I wasn't really sure how to understand that comment.
get_author_ids
FunctionI suggest to introduce a new function, get_author_idsthat would replace the
wp_get_post_termsand subsequent
array_map-
intvalcalls currently used twice in the
template.php` file.
This would make for easier unit testing, and it simplifies both the get_authors
and user_is_author
function.
π I addressed this in #40.
Just a note that I have not taken a look at the PHP tests, yet.
.gitignore
I like the alphabetical sorting. Maybe list folder names before files, though?
π I addressed this in #39.
Why is the composer.lock
file ignored? The plugin has a Composer production dependency, Asset Loader. Even though the package has an exact version contstraint, I suggest to not ignore the composer.lock
file.
Why is the package-lock.json
file ignored? The plugin has a few NPM production dependencies, including react-select
and react-sortable-hoc
. I suggest to not ignore the package-lock.json
file.
Is there anything preventing this from running on PHP 8? If not, I suggest to change the PHP dependency in the composer.json
file to "php": ">=7.2"
.
Just curious, why are you using exact version constraints for some dependencies?
Personally, I would default to non-breaking minors (i.e., using caret format, ^1.2.3
) and patch-level releases where I'm unsure about the nature of changes in individual releases (e.g., ~1.2.3
).
As far as I can see, the node-jq
package is only being used to read a property in the Composer config file, ... for a script inside that very file. This package requires quite a bit of overheadβusing WSL 2 on Windows, I had to install more than 130 MB of additional build tools to be able to build jq while running npm install
. I suggest to remove the node.jq
dependency and script, and hard-code the WordPress path in the composer.json
file twice.
π I addressed this in #39.
Are you planning on using HM Linter for Authorship?
If yes, I suggest to use exact version constraints for the HM Coding Standards, set to the same version that HM Linter would use. For the PHP Coding Standards, you already do this. For both JS and (S)CSS, you use ^1.1.1
.
There are some dependencies where I suggest to double-check that they are in the correct class, production vs. development-only dependencies:
@types/react-select
package be a development dependency?classnames
package be in dependencies
?The lint
script, running two commands concurrently, could do with a tiny bit of more information such as the file format (JS and CS, respectively), and maybe a bit of color.
π I addressed this in #39.
Consider adding <arg value="s"/>
to the config file, instead of specifying -s
on the command line (i.e., the test:phpcs
script in the composer.json
file). This is extra information that shouldn't negatively affect anything anyway, and, currently, both local usage and GitHub Actions already always print Sniff information.
π I addressed this in #39.
Do not lint the build/
folder.
π I addressed this in #39.
The PSR2R.Namespaces.UseInAlphabeticalOrder
sniff has been removed from the PSR2R ruleset. However, the HM Coding Standards are not quite up to date in terms of PSR2R version. I suggest to exclude the sniff for now.
π I addressed this in #39.
The PSR2R.Namespaces.UnusedUseStatement
sniff does not respect usage in PHP DocBlock or inline comments. Given that IDEs such as PhpStorm and VS Code require use
statements for symbols referenced in comments (in order to be able to navigate to the respective file), I suggest to exclude the sniff (and continue using symbols in non-FQN format).
π I addressed this in #39.
Also exclude the lib/
folder.
The code is using native ECMAScript modules, not CommonJS. So I suggest to configure TypeScript accordingly, for example, by using "module": "esnext"
and "moduleResolution": "node"
.
Personally, I like to import actual modules and functions, and not namespaces, so instead of this:
import * as React from 'react';
React.useState( /* ... */ );
I would use that:
import React, { useState } from 'react';
useState( /* ... */ );
By default, TypeScript will complain about the React
import, but you can add "allowSyntheticDefaultImports": true
to the tsconfig.json
file.
Why not allow fast failures?
What is (still?) failing when using Composer v2? Can we remove the composer:v1
directive for the "Install PHP2 step?
When installing PHP dependencies, we don't care about progress, and we can't handle any interactions, so let's use the --no-progress --no-interaction
flags.
Also, if you don't want tosee any regular output from Composer, you can use the -q
flag, which will then replace both --noprogress
and --no-suggest
, and it will even strip further output. Errors would still get printed to the console, so you would see them on GitHub.
π I addressed this in #39.
Both ESLint and stylelint are not executed on CI.
π I addressed this in #41.
For better coverage information, I suggest to tell PHPUnit where the source code lives. This is done by adding filter.whitelist
information.
π I addressed this in #39.
The PHPUnit config is missing XML schema information, and the current config is not valid. testsuites.testsuite.directory
does not support prefix
until PHPUnit 8. I suggest to remove the property, and use exclude
for undesired non-test files.
π I addressed this in #39.
ESLint is incorrectly set up. All JavaScript files are actually TypeScript files, having .ts
or .tsx
as extensions. By default, ESLint does not lint these files, even with the TypeScript parser and/or plugin configured.
Also, ESLint is outdated: currently v5, while v7 is the most recent version branch. In order for everything to work as expected, ESLint and select dependencies have to get udpated, and the config file, as well as the lint:js
script in the package.json
file needs updating, too.
Need a screenshot for the readme
Needs investigation. Similarly to RSS (see #32) Atom supports multiple <author>
elements but feed clients may only use the first. For example, the RSS widget in WordPress core only displays the first author from an Atom feed if there are multiple.
XML-RPC is unlike the REST API in that its routes are standardised so we can't change them to allow, for example, multiple authors for a post to be specified when a post is created or edited.
Subsequently I don't think it makes sense to try to expose multiple author information via XML-RPC either. What we do need is to decide exactly how XML-RPC should behave, and ensure it does. It might be that author information in XML-RPC should continue to correlate directly with the post_author
field.
We need a means of migrating an existing site that uses PPA.
Marking this as a P3 priority, not because it's not important but because we need the main structure of the plugin in place first.
User capability mapping so the capabilities of assigned authors acts exactly as if they are the post_author
.
Some examples that should be converted into test cases:
Author
meta box is hidden by default in the classic editor. Disable it completely.When a lower level user views the post editing screen for a post and they cannot change the author:
authorship
taxonomyauthor
Introduce a full CRUD endpoint in the REST API for guest authors.
This is needed because we're going to allow lower level users (eg. Authors) to create guest authors, and I don't want to mess about with the permissions of the /users
endpoint, and using a new endpoint abstracts this away from user storage.
Mostly expects and exposes the same fields as the /users
endpoint, except:
role
won't be required as the user will always have the guest author role/users
endpoint for non-authenticated requests, eg. by listing all guest authors that have published contentThe plan is to map a user to a post via a term. This is similar to how CAP and PPA work, except the term will not contain any user-facing data and instead reference a user ID directly via its name
and/or slug
field.
If we imagine that the WordPress database used foreign keys, then the structure would look like this:
ββββββββββββββββββ ββββββββββββββββββ ββββββββββββββββββ ββββββββββββββββββ ββββββββββββββββββ
β wp_posts β β wp_term_ β β wp_term_ β β wp_terms β β wp_users β
β β β relationships β β taxonomy β β β β β
ββββββββββββββββββ ββββββββββββββββββ ββββββββββββββββββ ββββββββββββββββββ ββββββββββββββββββ
ββββββββββββββββββ ββββββββββββββββββ ββββββββββββββββββ ββββββββββββββββββ ββββββββββββββββββ
β ID βββββββ€ object_id β ββββ€term_taxonomy_idβ ββββ€ term_id β ββββ€ ID β
ββββββββββββββββββ ββββββββββββββββββ€ β ββββββββββββββββββ€ β ββββββββββββββββββ€ β ββββββββββββββββββ
βterm_taxonomy_idββββ β term_id ββββ β slug ββββ
ββββββββββββββββββ ββββββββββββββββββ ββββββββββββββββββ
β²
β
β
βββββββββ»ββββββββ
β Term βββΆ User β
βββββββββββββββββ
We need to do a bit of investigation into whether storing a numeric ID in the name or slug field of a term works as expected. For example, give a term with term_id => 1
and slug => 2
:
2
or will WordPress internally convert that to a query for the term_id
?WP_Term_Query
works as expected now and we're all good.This is an idea that most likely will not be implemented due to the low value compared to its high complexity, but it serves as a place to see the thought process and discuss the idea in the future.
Along with supporting multiple attributed authors for a post, it should be possible to support multiple editorial authors for a post. An editorial author is an author who can edit the post but is not necessarily attributed.
A post where multiple Author or Contributor level users need to be able to edit the post but not be attributed. Useful for a future collaborative editing workflow.
post_author
field. This allows for some level of compat with plugins which read/write to post_author
directly, and also allows for sane behaviour when the plugin is deactivated.If an Administrator or Editor places some JavaScript into the content of a post and then adds an Author or Contributor as an attributed author, the JavaScript will be stripped out if the Author or Contributor make an edit to the post. This is due to the unfiltered_html
capability.
We mustn't change this functionality, but it ought to be documented so users are clear about what happens in this situation.
WP_Query
to override the standard author_name
query var and convert it to use a term query so front end author archives work - should only affect post types that support author
WP_Query
to override the standard author
query var and convert to a term query so author filtering in the admin area works - should only affect post types that support author
Need to document what happens when the plugin is deactivated.
remove_role()
When any of the attributed authors of a post leave a comment on it, the comment should get the bypostauthor
class added so it can be highlighted by the theme if necessary.
When the component first mounts, and every time it remounts without the user list having been changed, we can pull the current list of user obects from the _embedded
attribute of the post instead of via the REST API. This will save REST API requests.
Need to add support for embedding the user objects when the _embed
query parameter is used.
/posts
/post/X
/pages
/pages/X
_links
element containing a corresponding array of user linksauthor
for a consistent interface when reading dataWhen a post is updated, changes to the attributed authors need to be recorded in audit trail plugins.
The three big ones are:
When the Authorship plugin is not in use and the author of a post is changed:
Tasks:
If the site is set up to send email notifications for newly published comments (under Settings -> Discussion -> Email me whenever -> Anyone posts a comment), all of the post authors with a valid email address should receive the comment notification email.
The "Author" field should not be shown in Quick Edit and Bulk Edit, pending later development of a feature that enables this.
Introduce granular user capability mapping for controlling:
When a post is attributed to multiple authors they should all be displayed in the RSS feed item for that post. WordPress uses the Dublin Core namespace for author information in the <dc:creator>
element.
The RSS spec is famously vague. The RSS Best Practices Profile says An item MAY contain more than one dc:creator element to credit multiple authors
but in practice it seems that RSS clients only display the first. For example SimplePie fully supports multiple <dc:creator>
elements but the RSS widget in WordPress core will only display the first.
This means that multiple authors should all be listed in the same <dc:creator>
element.
The data for this element comes from the_author()
so this might be implemented at that level. Needs investigation as the_author()
is used in many places.
These issues apply to PublishPress Authors, but mostly affect Co-Authors Plus too.
\MultipleAuthors\Classes\Objects\Author
as a "fake" user object that's compatible with WP_User
Haven't looked into this yet but after you choose some authors and then switch the sidebar to another tab, such as the Block tab, when you switch back the selected authors are reverted.
If the site is set up to send email notifications for comment moderation (under Settings -> Discussion -> Email me whenever -> A comment is held for moderation), any of the post authors who have the capability to moderate the comment should also receive the comment moderation email.
Following on from #6, add two more basic template functions for displaying a list of authors for a post:
In both cases the display name for each author should be used, and the link for each author should link to their author archive.
The ability to assign attributed authors to a post is directly connected to whether the post type supports author
. These checks should be replaced with calls to a function which encapsulates this check and a filter on its return value, so the feature can be enabled and disabled for all post types without necessarily affecting the post type support.
Maybe even check for a custom post type support of authorship
too.
We could leave humanmade/asset-loader
as a dependency, but this may cause problems for projects which are currently using an older version. Its API functions changed in 0.4.0.
Alternatively we could bundle it. Need to make a decision.
Similarly to #16 we need a new REST API endpoint for searching and listing all authors from the context of a user who can assign authors to a post.
We need this because we'll allow lower level users to search authors and I don't want to change the permissions of the /users
endpoint.
/users
unauthenticated to fetch a list of users with published content.Same problem as johnbillion/query-monitor#551 . We need a mechanism to build the CSS and JS assets for deployment via Composer, and potentially for WordPress.org if we decide to release it there.
Probably go for the same approach, where a GitHub Action builds the plugin into a separate branch and tags it.
Via the Creatable
feature of React Select.
author
ID for the columnFollows on from #9.
Need to update all other author
-related query variables, eg. author__in
, and convert them to a term query.
As a result of #9 the "Mine" link on the post listing screen will switch to showing posts that you're an assigned author of. We therefore need to either:
post_author
field), orAlso, really need to nail the terminology. Too many authors.
The underlying component(s) that we use for the author selection control on the post editing screen will hopefully be tested, but it would be great to have functional tests in place to ensure it actually works in the context of where we use it (the block editor in #7 and the classic editor in #13). Suggestions for testing approach and framework welcome.
Similarly to #16 , a convenience command for adding a guest author should be added to WP-CLI which has the same list of required and optional fields as its REST API endpoint counterpart.
Need to check that the author archives in XML sitemaps work as expected.
Implement multiple author selector on the block editor post editing screen
/wp/v2/users
endpoint for nowwp_localize_script()
or via the authorship
property of the post in the REST API (#5) as this is preloaded with the API middleware on the editing screenThe get_the_archive_title()
function in WordPress core uses the first post in its results as the basis for the title that it constructs for the heading on archives. It should instead use the queried user.
The effect this has is that until we decide how we're handling the $authordata
global, this title needs to be manually fixed so it uses the actual queried user on author archives as it currently shows the incorrect user display name if the queried author isn't the same as the post_author
of the first post in the results.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
π Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. πππ
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google β€οΈ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.