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

Fix vote ordering #15655

Merged
merged 3 commits into from May 5, 2017

Conversation

Projects
None yet
10 participants
@Bakual
Contributor

Bakual commented Apr 28, 2017

Pull Request for Issue #15549.

Summary of Changes

Changes the articles and featured models back to using the list.ordering state instead of the list.fullordering one. Added the new ordring options to the whitelist.

This also removes the check for the vote plugin from JFormFieldList because that class certainly shouldn't he to deal with that. Instead I created a new custom formfield class for com_content. The behavior of the respective parameters is exactly the same as before.

Testing Instructions

  • Test that ordering in the article and featured article manager works. Article manager by default should be ID Descending, featured one title ascending.
  • Test that sorting by votes and ratings works.

Expected result

Works

Actual result

Default is ID ascending for the article manager.

Documentation Changes Required

None. This restores previous behavior.

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Apr 28, 2017

I have tested this item successfully on e570d2d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15655.

I have tested this item successfully on e570d2d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15655.

@sseguin613

This comment has been minimized.

Show comment
Hide comment
@sseguin613

sseguin613 Apr 28, 2017

Test that ordering in the article and featured article manager works. Article manager by default should be ID Descending. (This is a quote from the top of this page).

It is displaying by default in the article manager drop down selector as ID Descending ,but the actual sorting is ID Ascending.
capture

sseguin613 commented Apr 28, 2017

Test that ordering in the article and featured article manager works. Article manager by default should be ID Descending. (This is a quote from the top of this page).

It is displaying by default in the article manager drop down selector as ID Descending ,but the actual sorting is ID Ascending.
capture

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Apr 28, 2017

Contributor

Hmm, that's strange, because when I try it it defaults to ID descending now both in the list and in the dropdown.

Contributor

Bakual commented Apr 28, 2017

Hmm, that's strange, because when I try it it defaults to ID descending now both in the list and in the dropdown.

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Apr 28, 2017

Contributor

@sseguin613 Did you apply this PR before trying?

Contributor

Bakual commented Apr 28, 2017

@sseguin613 Did you apply this PR before trying?

@sseguin613

This comment has been minimized.

Show comment
Hide comment
@sseguin613

sseguin613 Apr 29, 2017

I just applied the PR again and it is working as it should. I must have missed a file. Thank you for the quick fix. I can confirm it works as intended.

I just applied the PR again and it is working as it should. I must have missed a file. Thank you for the quick fix. I can confirm it works as intended.

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Apr 29, 2017

Contributor

@sseguin613 When you have tested it successfully, please mark it here. Thanks.
https://issues.joomla.org/tracker/joomla-cms/15655

Contributor

Quy commented Apr 29, 2017

@sseguin613 When you have tested it successfully, please mark it here. Thanks.
https://issues.joomla.org/tracker/joomla-cms/15655

@sseguin613

This comment has been minimized.

Show comment
Hide comment
@sseguin613

sseguin613 Apr 29, 2017

I have tested this item successfully on e570d2d

Applied PR and issue corrected and functioning as intended.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15655.

I have tested this item successfully on e570d2d

Applied PR and issue corrected and functioning as intended.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15655.

@ghazal

This comment has been minimized.

Show comment
Hide comment
@ghazal

ghazal Apr 29, 2017

Contributor

I have tested this item successfully on e570d2d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15655.

Contributor

ghazal commented Apr 29, 2017

I have tested this item successfully on e570d2d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15655.

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Apr 29, 2017

RTC after 3 successful tests.

RTC after 3 successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC label Apr 29, 2017

@photodude

This comment has been minimized.

Show comment
Hide comment
@photodude

photodude Apr 29, 2017

Contributor

@franz-wohlkoenig I don't believe it can be RTC when it's failing the code style check on drone.

Contributor

photodude commented Apr 29, 2017

@franz-wohlkoenig I don't believe it can be RTC when it's failing the code style check on drone.

@rdeutz rdeutz added this to the Joomla 3.7.1 milestone May 4, 2017

@rdeutz rdeutz merged commit 744f466 into joomla:staging May 5, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@joomla-cms-bot joomla-cms-bot removed the RTC label May 5, 2017

@gwsdesk

This comment has been minimized.

Show comment
Hide comment
@gwsdesk

gwsdesk May 10, 2017

I have tested this item successfully on e570d2d

gwsdesk commented May 10, 2017

I have tested this item successfully on e570d2d

brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request May 16, 2017

Fix vote ordering (#15655)
* Fixing ordering by rating and rating_count (don't use fullordering!)

* Removing logic to check for the vote plugin from the JFormFieldList and add a custom formfield to handle that specific case.

* Update votelist.php
@nigelbpeck

This comment has been minimized.

Show comment
Hide comment
@nigelbpeck

nigelbpeck May 17, 2017

Great to have this fixed (and so quickly). Thanks all.

Great to have this fixed (and so quickly). Thanks all.

@Bakual Bakual deleted the Bakual:FixVoteOrdering branch Jun 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment