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

Review permalink issue #253

Merged
merged 39 commits into from
Jun 3, 2021
Merged

Conversation

NeilWJames
Copy link
Collaborator

The problem with the multisite test was actually due to the environment having plain permalink style. The normal style is /year/month/post_name/.
Plain results in a name like .../post_type=document?p=7
For a revision this was .../post_type=document?&p=7&revision=1.txt (for a text file loaded)

Code has been changed to be ../post_type=document?&p=7&revision=1 which will work.

The admin tests have been changed to incorporate running tests with Plain and Year/Month/Post_name styles.

Separately I have extended the rewrite matching rules to not require the year/month and or the file extension. My logic for doing this is that standard WP will find the document if you enter ....index.php?document=post_name so allowing .../documents/post_name is the same really.

I have made workflow_state appear on the admin list of documents using standard WP functionality and updated the Read mes.

I have updated the code coverage badge too.

Neil James

Merge pull request wp-document-revisions#250 from NeilWJames/master
This represents aligning to the wp-die-fix branch whilst remaining with travis and not github actions.
The contents of test is simply the code in the wp-die-fix branch.
Match if year/month and/or file extension not entered.

Will match with just the post name - just like a post,
@NeilWJames NeilWJames requested a review from benbalter June 3, 2021 19:36
@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2021

Codecov Report

Merging #253 (9e98713) into master (32b2aec) will increase coverage by 0.92%.
The diff coverage is 76.47%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #253      +/-   ##
============================================
+ Coverage     44.84%   45.76%   +0.92%     
+ Complexity      628      625       -3     
============================================
  Files             7        7              
  Lines          2143     2137       -6     
============================================
+ Hits            961      978      +17     
+ Misses         1182     1159      -23     
Flag Coverage Δ
multisite- 43.08% <84.61%> (+0.94%) ⬆️
multisite-0 45.39% <76.47%> (+0.92%) ⬆️
multisite-1 43.67% <84.61%> (+0.94%) ⬆️
php-5.6 45.26% <76.47%> (+0.85%) ⬆️
php-7 44.01% <84.61%> (+0.94%) ⬆️
php-7.1 43.63% <84.61%> (+0.94%) ⬆️
php-7.2 43.57% <84.61%> (+0.94%) ⬆️
php-7.3 43.62% <84.61%> (+0.94%) ⬆️
php-7.4 43.67% <84.61%> (+0.94%) ⬆️
wp- 43.67% <84.61%> (+0.94%) ⬆️
wp-5.0 43.08% <84.61%> (+0.94%) ⬆️
wp-latest 45.39% <76.47%> (+0.92%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
includes/class-wp-document-revisions-front-end.php 57.48% <0.00%> (-0.28%) ⬇️
includes/class-wp-document-revisions-admin.php 23.66% <75.00%> (+2.79%) ⬆️
includes/class-wp-document-revisions.php 53.20% <100.00%> (+0.13%) ⬆️

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 32b2aec...9e98713. Read the comment docs.

Copy link
Collaborator

@benbalter benbalter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this is great. I especially like how you broke up the plain vs pretty permalink tests. Definitely a lot more confident about permalink handling going forward given the challenge folks have been facing in the forum - thank you.

@benbalter
Copy link
Collaborator

And thank you for all your housekeeping documentation! Much appreciated! 🙇🏻

@benbalter
Copy link
Collaborator

I can go ahead and get this merged. Given that there's so many great features and fixes in 3.3, I can take a pass at cleaning up the docs as well, to ensure they're complete/up to date, and can try to get a new release out later today or tomorrow.

@benbalter benbalter merged commit f28faf9 into wp-document-revisions:master Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants