Set author display name on Post / Page creation, send author changes to API.

[twstokes]

This is one PR in a series towards implementing #9015.

This PR is branched from #16298, so it should be merged first.

Affected areas:

This PR aims to accomplish two things:

1. Sending an updated authorID when an author is changed for a Post or Page.
   Why: To support a future PR (Feature: Allow the user to change the author for multi-author sites for Posts and Pages #16281) we want the value of the author user ID to be sent to the API. Currently it's not sent. This PR updates the RemotePost's authorID value so that it will be picked up by WordPressKit (see PR: Include Author ID in Post Service parameters. WordPressKit-iOS#381) and ultimately sent to the API. REST API reference 1, reference 2, XML-RPC API reference 1 (see wp.newPost and wp.editPost).

2. Setting an author user ID and display name on a newly created Post or Page.
   Why: To support a future PR (Feature: Allow the user to change the author for multi-author sites for Posts and Pages #16281) we want the author display name and author user ID to be known on Post or Page creation to allow changing an author for a multi-user blog. We must know both so that the user interface can show the author's name (by default, theirs), and showing the currently selected author (which is determined by the author ID) in the author list. Currently, neither of these are set.

The ability to change an author in the UI will be provided in an upcoming PR: #16281

To test:

Local changes to the author

• Run the unit tests PostsTests. Manipulating this from the app interface isn't currently possible, but will be in Feature: Allow the user to change the author for multi-author sites for Posts and Pages #16281.

Setting the authorID and author name on Post and Page creation

1. Add breakpoints on EditPageViewController:56 and EditPostViewController:127.
2. Create a new Post or Page using the app.
3. When execution is paused for debugging, observe the value of the Post or Page (newPost or newPage).
4. Both the newPost and newPage should have a populated author and authorID field.

The authorID is now sent on Post / Page creation and modification

1. Add a breakpoint on PostService.m:266.
2. Create or update a Post or Page.
3. Observe that the value for remotePost contains the authorID.

Regression Notes

1. Potential unintended areas of impact

• Creating or updating posts or pages.

2. What I did to test those areas of impact (or what existing automated tests I relied on)

• Created a new Post on a WordPress.com and self-hosted blog.
• Updated a Post on a WordPress.com and self-hosted blog.

3. What automated tests I added (or what prevented me from doing so)

• PostTests - testLocalChangesWhenAuthorIsChanged
• PostTests - testLocalChangesWhenAuthorIsTheSame

PR submission checklist:

• I have completed the Regression Notes.
• I have considered adding unit tests for my changes.
• I have considered adding accessibility improvements for my changes.
• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[guarani]

Thanks for catching this bug where self-hosted post authors don't show up properly in the post list. Would this be best addressed in a new issue and this PR target develop instead of the feature branch?

I smoke tested this on a self-hosted site and it looks to fix the issue on the post list screen of authors not showing up. I had trouble testing the effects to the edit history screen, I couldn't get the History option to appear in the bottom sheet in the editor when I tap the three-dot menu. I might be missing something, did you run into this?

---

A few comments on the PR itself:

This PR may help resolve a UI bug where a WordPress.org site doesn't show the author in the list of posts when viewing for "Everyone", and the user is an Administrator.

If this bug is independent from #9015, that would be a point in favor of making a separate issue for this bug.

I'm not clear from the PR whether this bug affects only self-hosted (WordPress.org) sites, or also affects WP.com (a separate issue for this bug could help us triage it).

Could the testing steps be more descriptive to specify what to expect with regard to the Post list screen?

Just a few nitpicks:

• Is there a PR title that better describes the issue?
• The screenshots on the PR are very large (I sometimes use a HTML img tag to adjust the width to something like 350px).

[twstokes]

Thanks for catching this bug where self-hosted post authors don't show up properly in the post list. Would this be best addressed in a new issue and this PR target develop instead of the feature branch?

I think that's a great idea, will do.

I smoke tested this on a self-hosted site and it looks to fix the issue on the post list screen of authors not showing up. I had trouble testing the effects to the edit history screen, I couldn't get the History option to appear in the bottom sheet in the editor when I tap the three-dot menu. I might be missing something, did you run into this?

I ran into this as well, and assumed it's not a feature of self-hosted sites. I'll look into that some more. 👍

If this bug is independent from #9015, that would be a point in favor of making a separate issue for this bug.

Agreed. It was a "nice bonus" that appeared, but I'll make it the main driver for the fix.

I'm not clear from the PR whether this bug affects only self-hosted (WordPress.org) sites, or also affects WP.com (a separate issue for this bug could help us triage it).

Could the testing steps be more descriptive to specify what to expect with regard to the Post list screen?

Sure - I'll make this clear in the dedicated bug issue for it.

• Is there a PR title that better describes the issue?

Agreed, will improve that.

• The screenshots on the PR are very large (I sometimes use a HTML img tag to adjust the width to something like 350px).

Good call, will do. Thanks!

[twstokes]

Thanks for catching this bug where self-hosted post authors don't show up properly in the post list. Would this be best addressed in a new issue and this PR target develop instead of the feature branch?

I smoke tested this on a self-hosted site and it looks to fix the issue on the post list screen of authors not showing up. I had trouble testing the effects to the edit history screen, I couldn't get the History option to appear in the bottom sheet in the editor when I tap the three-dot menu. I might be missing something, did you run into this?

A few comments on the PR itself:

This PR may help resolve a UI bug where a WordPress.org site doesn't show the author in the list of posts when viewing for "Everyone", and the user is an Administrator.

If this bug is independent from #9015, that would be a point in favor of making a separate issue for this bug.

I'm not clear from the PR whether this bug affects only self-hosted (WordPress.org) sites, or also affects WP.com (a separate issue for this bug could help us triage it).

Could the testing steps be more descriptive to specify what to expect with regard to the Post list screen?

Just a few nitpicks:

• Is there a PR title that better describes the issue?
• The screenshots on the PR are very large (I sometimes use a HTML img tag to adjust the width to something like 350px).

The PR should be updated now to address these issues. Thanks!

[peril-wordpress-mobile]

You can trigger an installable build for these changes by visiting CircleCI here.

[guarani]

This code should have no effect on the current app.

Would it be possible to have testing steps, even if they involve diving into code in the IDE (e.g. putting breakpoints on certain lines and printing values, etc.)?

Also, could you please link to the API documentation showing where these parameters are expected when publishing a post (i.e. author display name, etc)? At first glance it seems like a post wouldn't need more than an authorID, but I can imagine that it might need display name in case the author is then deleted. Just want to corroborate with the API documentation here.

[twstokes]

This code should have no effect on the current app.

Would it be possible to have testing steps, even if they involve diving into code in the IDE (e.g. putting breakpoints on certain lines and printing values, etc.)?

Also, could you please link to the API documentation showing where these parameters are expected when publishing a post (i.e. author display name, etc)? At first glance it seems like a post wouldn't need more than an authorID, but I can imagine that it might need display name in case the author is then deleted. Just want to corroborate with the API documentation here.

Hi @guarani, thanks for the feedback! I've updated the description of the PR to hopefully fit these suggestions, but please let me know if I need to expand any areas.

[guarani]

I bumped the milestone to 17.3 since I just realized that there's no reason (I can think of) to try to get this into 17.2 which is cut Monday (something I'd overlooked when adding a milestone earlier).

[guarani]

Thanks for the updates, @twstokes!

I see the build tests are failing:

Are you able to look into that? If you don't have access, or there's an error that you don't recognize, just let me know and I can help.

[twstokes]

Thanks for the updates, @twstokes!

I see the build tests are failing:

Are you able to look into that? If you don't have access, or there's an error that you don't recognize, just let me know and I can help.

I saw that @guarani, and it looks like xcodebuild timed out. Since the command timed out rather than throwing a compilation error, I wonder if CircleCI had a hiccup?

Do you have the ability to try a re-run? It doesn't look like I'm privy to that. Thank you!

[twstokes]

Thanks for the updates, @twstokes!
I see the build tests are failing:

Are you able to look into that? If you don't have access, or there's an error that you don't recognize, just let me know and I can help.

I saw that @guarani, and it looks like xcodebuild timed out. Since the command timed out rather than throwing a compilation error, I wonder if CircleCI had a hiccup?

Do you have the ability to try a re-run? It doesn't look like I'm privy to that. Thank you!

Actually @guarani let me resolve these merge conflicts, first. Thanks!

[peril-wordpress-mobile]

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

[guarani]

I tested on a WP.com site and LGTM! From your comment here sounds like we won't merge this until Tuesday.

[twstokes]

Excellent. Thank you @guarani!

[guarani]

Just a heads-up that the 17.2 branch is now cut: https://github.com/wordpress-mobile/WordPress-iOS/tree/release/17.2
It should be safe to get this merged to develop for inclusion in 17.3.

[twstokes]

Sounds good @guarani, I'll resolve the merge conflict by pointing to the appropriate WordPressKit and push the changes.

[guarani]

I noticed the changes to the Podfile were reverted in 4f50606. I think my comment in wordpress-mobile/WordPressKit-iOS#381 (review) might have been misleading. With the second option I mentioned there, we still need to tag a new WordPressKit beta release after merging its PR and we then need to point this PR to it, and merge this.

I've gone ahead and made the release here: https://github.com/wordpress-mobile/WordPressKit-iOS/releases

When this PR is updated to point to the tag, I can trigger the CI to run again and we can get this merged 👍

[twstokes]

Sorry I misunderstood @guarani! I believe this is now resolved with the latest push.

[guarani]

I've enabled auto-merge and re-triggered the CI, so with a bit of luck this should get automatically merged once CI has passed.