wp-cli / entity-command

Manage WordPress comments, menus, options, posts, sites, terms, and users.
MIT License
100 stars 89 forks source link

Add site generator command #498

Closed i-am-chitti closed 4 months ago

i-am-chitti commented 4 months ago

Description

Add site generator command

wp site generate

Options

Issues

Fixes #231 Related https://github.com/wp-cli/wp-cli/issues/5935

swissspidy commented 4 months ago

Neat!

With this being a new command, let's add it to this list here:

https://github.com/wp-cli/entity-command/blob/8b19b7dacca26dea8f675e394230c63ebee1277d/composer.json#L125-L130

That will make sure the readme can be updated properly. The readme will be updated automatically via a cron workflow, but you can also update it manually in this PR using wp scaffold package-readme if you'd like to see the changes.

i-am-chitti commented 4 months ago

@swissspidy Addressed the feedback. Please have a re-review. Also, the Behat tests are failing for the newly added site-generate.feature . It's passing on my local -

image

In CI -

image
swissspidy commented 4 months ago

I don't see that test failing on CI, I only see this error:

PHP Fatal error:  Call to undefined function get_subdirectory_reserved_names() in src/Site_Command.php on line 555

That's because of my suggestion to use get_subdirectory_reserved_names(). Turns out this was only added in WP 4.4, but we still run tests against 3.7+.

To address this, we can add a new private method to the class like this:

/**
 * Retrieves a list of reserved site on a sub-directory Multisite installation.
 *
 * Works on older WordPress versions where get_subdirectory_reserved_names() does not exist.
 *
 * @return string[] Array of reserved names.
 */
private function get_subdirectory_reserved_names() {
    if ( function_exists( 'get_subdirectory_reserved_names' ) ) {
        return get_subdirectory_reserved_names();
    }

    $names = array(
        'page',
        'comments',
        'blog',
        'files',
        'feed',
        'wp-admin',
        'wp-content',
        'wp-includes',
        'wp-json',
        'embed',
    );

    // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Calling WordPress native hook.
    return apply_filters( 'subdirectory_reserved_names', $names );
}
swissspidy commented 4 months ago

Ah, now I see the failing test correctly, again only on older WP versions. https://github.com/wp-cli/entity-command/actions/runs/8852432605/job/24311451054?pr=498#step:11:86

Unfortunately I don't have a local 3.7 test site (or an old PHP version) right now to manually test this.

I wonder if it has to do with the URL scheme (https vs http). Some tests do

    And I run `wp site list --site__in={SITE_ID} --field=url | sed -e's,^\(.*\)://.*,\1,g'`
    And save STDOUT as {SCHEME}

And then use e.g. {SCHEME}://example.com/site1/ in the test.

If it's not that, then maybe someone else has an idea.


Aside: any idea why this one other unrelated test could be failing only on this PR?

https://github.com/wp-cli/entity-command/actions/runs/8852432605/job/24311451385?pr=498#step:11:84

ernilambar commented 4 months ago

@swissspidy https://github.com/wp-cli/entity-command/actions/runs/8852432605/job/24311451385?pr=498#step:11:84

Regarding this error, I have also seen this while working in the PR which was merged yesterday.

We have:

When I run `wp user application-password create 1 myapp1`
Then STDOUT should not be empty

When I run `wp user application-password create 1 myapp2 --app-id=42`
Then STDOUT should not be empty

We are creating two application passwords here. If the timestamp is exactly same for both entries then order of entries is not predictable for following command. And then this error appears.

wp user application-password list 1 --format=ids --orderby=created --order=asc

Should we use sleep command before creating second application password in the test so that timestamp for two entries here will never be same?

swissspidy commented 4 months ago

I see.

We should avoid things likesleep.

There must be other ways to make the test more robust. For example:

a) remove the orderby=created test case as it doesn't seem to provide too much value b) rewrite the test to do something more like this:

When I run `wp user application-password list admin --orderby=created --order=asc --format=csv`
Then STDOUT should contain:
    """
    {UUID1},,myapp1,{HASH1},{CREATED1},,,,
    """
And STDOUT should contain:
    """
    {UUID1},42,myapp12{HASH2},{CREATED2},,,,
    """

Wanna open a PR?

ernilambar commented 4 months ago

@swissspidy https://github.com/wp-cli/entity-command/pull/499

swissspidy commented 4 months ago

@wp-cli/committers Does anyone have thoughts here on how to debug the WP 3.7 failure? I don't have a working local setup to test it myself.

ernilambar commented 4 months ago

Looks like we faced same issue and fixed using {SCHEME} few years ago. https://github.com/wp-cli/entity-command/commit/e0a00c555f6a4b1ab467460b2c306aaa5b2157b7 May be we should use same approach here also.

i-am-chitti commented 4 months ago

Thanks @ernilambar. This is fixing the failing tests on local. Hoping to get the same results in CI.

ernilambar commented 4 months ago

@swissspidy Any idea why regenerate-readme was not triggered after this PR?