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

Add revision display tests and fix issues #116

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

phy25
Copy link
Contributor

@phy25 phy25 commented Dec 25, 2017

This is a more well-organized patch replacing PR #98, after tests are split and carefully compared between code branches.

To sum up what these commits do:

  1. Add basic "show expected version" tests. They were written based on definitions at https://www.dokuwiki.org/plugin:publish#display_revision. And it failed because:

There were 2 failures:

  1. approvel_test::test_show_expected_revision
    Visiting a draft revision with AUTH_READ did not return draft content.
    Failed asserting that false is true.
    /home/travis/build/phy25/dokuwiki-plugin-publish/lib/plugins/publish/_test/publish.test.php:267
  2. approvel_test::test_show_expected_revision_hide_drafts
    Visiting a draft revision with hide_drafts on did not return in denied mode.
    Failed asserting that false is true.
    /home/travis/build/phy25/dokuwiki-plugin-publish/lib/plugins/publish/_test/publish.test.php:286

FAILURES!
Tests: 36, Assertions: 104, Failures: 2.

  1. Fix "visiting a draft revision with AUTH_READ did not return draft content", In the test case it is showing the latest version (redirected). So don't redirect to the latest revision if [specific] [draft] revision is requested. After these commits, tests error reduced:

There was 1 failure:

  1. approvel_test::test_show_expected_revision_hide_drafts
    Visiting a draft revision with hide_drafts on did not return in denied mode.
    Failed asserting that false is true.
    /home/travis/build/phy25/dokuwiki-plugin-publish/lib/plugins/publish/_test/publish.test.php:286

FAILURES!
Tests: 36, Assertions: 104, Failures: 1.

  1. Fix "visiting a draft revision with hide_drafts on did not return in denied mode". Here we have to detect user permission based on REV instead of ID, because of the "hide drafts" feature. The code is somewhat inefficient indeed because I tried to keep consistence of the function usage in old parts of the code, so this may be optimized later.
    Whatever, it works.

OK (36 tests, 105 assertions)

- add DOKU_UNITTEST check against static variable cache to make tests run
- Fix error "Visiting a draft revision with AUTH_READ did not return draft content"
- fixes "Visiting a draft revision with hide_drafts on did not return in denied mode"
@phy25
Copy link
Contributor Author

phy25 commented Mar 13, 2019

Without this PR the tests will always fail now, in 2019. I don't quite remember why I mixed a lot of other non-essential bugfix into this PR, but please review this when you have time. Thanks!

@phy25
Copy link
Contributor Author

phy25 commented Mar 14, 2019

There's an issue after applying this patch: with $conf['plugin']['publish']['hide drafts'] = 1;$conf['plugin']['publish']['author groups'] = 'admin';, when a user out of author groups with edit permission looks at a page with currently not-approved version and previous approved version, it doesn't show the approved version, but showing denied message instead.

It might be because handle_start redirect code won't handle requests from users with edit access.

if ($INFO['perm'] != AUTH_READ) {
return;
}

A previous commit shows that this line makes redirect "apply to readers only". Honestly I suddenly don't know what to do with so many unclear behavior from this plugin.

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.

1 participant