assertTaxonomyPage in node.test has no documentation.

CommentFileSizeAuthor
#16 1318316-d7-16.patch1.7 KBxjm
#11 1318316-11.patch1.72 KBxjm
#11 interdiff.txt786 bytesxjm
#6 1318316-5.patch1.55 KBxjm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Priority: Major » Normal

Methods in tests missing docs is the exact opposite of a major bug.

joachim’s picture

joachim CreditAttribution: joachim commented

Seems to me if the tests with which you check the code is working themselves are not understandable, we have a problem multiplied.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Component: node.module » taxonomy.module
Category: bug » task
Priority: Normal » Major

That's still not a functional bug, but I'll split the difference and do this?

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Assigned: Unassigned » xjm
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Active » Needs review

Added a doclock. I also took the liberty of renaming $super to $is_admin. (Even though it was $super.) :)

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
FileSize
1318316-5.patch1.55 KB

How about I upload the patch.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Component: taxonomy.module » documentation

Really this is docs more than taxonomy.module itself.

joachim’s picture

joachim CreditAttribution: joachim commented
Status: Needs review » Reviewed & tested by the community

Test bot passes, docs look good, and the renamed param is definitely a good thing.

chx’s picture

chx CreditAttribution: chx commented

The patch is super!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Needs review 
// Non-superusers on the private page can only see their own nodes.

Should this be "non-admins" now the variable is changed? Super user is usually uid 1 for us.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
FileSize
interdiff.txt786 bytes
1318316-11.patch1.72 KB

With comment correction. I double-checked and this eliminates everything super from the file. Sorry node.test.

Status: Needs review » Needs work

The last submitted patch, 1318316-11.patch, failed testing.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs work » Needs review

Bot issue.

Tor Arne Thune’s picture

Tor Arne Thune CreditAttribution: Tor Arne Thune commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/pushed to 8.x, thanks!

Seems like this could be backported.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Patch (to be ported) » Reviewed & tested by the community
Issue tags: +Needs backport to D7
FileSize
1318316-d7-16.patch1.7 KB

Straight reroll for D7.

I also switched the backport tag to what (I think?) is the standard one, at least from http://drupal.org/node/1207020. (Maybe someone with database access should re-tag all nodes for the former with the latter and then delete it.)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Assigned: xjm » Unassigned