Notification content

[danbp]

As already explained on #6205, the content doesn't go to next page when on the notification screen.

Tested with BP 2.2.1 MS, and 2013 - no other plugin installed.

This is a different problem and why i open a new ticket. Apologize if i doing it wrong.

[johnjamesjacoby]

Moving to 2.2.2 to investigate further.

Likelihood of this being a regression seems small, as I don't think this code was modified in such a way to cause breakage it during the 2.2 cycle.

[johnjamesjacoby]

Fix & Test

[johnjamesjacoby]

​6229.01.patch includes a fix and unit test to support results of BP_Notifications_Notification::get() return the correctly paginated results.

It also adds a query_vars variable to BP_Notifications_Template which is a bit redundant, but it proved helpful for passing the same array to both BP_Notifications_Notification::get() and BP_Notifications_Notification::get_total_count() and for keeping pristine the variables used to translate the request into queried results. Since no other components work this way, I'm open to removing it and using a locally scoped array instead, but I thought it was a neat so included it here.

[boonebgorges]

Fix looks OK to me. For anyone following along, the issue is that BP_Notifications_Template is parsing the 'npage' URL param, but is not then passing it along properly to BP_Notifications_Notification::get(). johnjamesjacoby's new variable properly includes the 'page' parameter.

I'm indifferent about the query_vars bit. If you like it, roll with it.

See 6229.02.diff​ for updated tests. The test you originally added is pretty good (though I made it more precise - as it stood, you were only testing the number of items being returned, but that doesn't tell us whether the correct page of results has been returned, at least not directly). However, the test you wrote doesn't actually describe the bug - it passes even without your patch to BP_Notifications_Template. I have added an additional test in 02.diff that does demonstrate the fix.

This is not a regression but it's a pretty lousy bug, so 2.2.2 is fine if no one else has objections.

[johnjamesjacoby]

In 9570:

Notifications: Ensure pagination works with $_GET parameters.

Props boonebgorges. See #6229. (trunk)

[johnjamesjacoby]

In 9571:

Notifications: Ensure pagination works with $_GET parameters.

Props boonebgorges. See #6229. (2.2 branch)

[johnjamesjacoby]

Thank you Boone for the updated patches.

Marking as fixed.

[danbp]

It works ! Merci beaucoup Boone and John.