From ae871c4d46a8498a3ff5c400baa6e82174b384d0 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Mon, 31 Aug 2020 19:47:09 +0900 Subject: Make Array-creation behavior of Paginable more predictable (#14687) * Make Array-creation behavior of Paginable more predictable Paginable.paginate_by_id usually returns ActiveRecord::Relation, but it returns an Array if min_id option is present. The behavior caused problems fixed with the following commits: - 552e886b648faa2a2229d86c7fd9abc8bb5ff99c - b63ede5005d33b52266650ec716d345f166e2df0 - 64ef37b89de806f49cc59e011aa0ee2039c82c46 To prevent from recurring similar problems, this commit introduces two changes: - The scope now always returns an Array whether min_id option is present or not. - The scope is renamed to to_a_paginated_by_id to clarify it returns an Array. * Transform Paginable.to_a_paginated_by_id from a scope to a class method https://api.rubyonrails.org/classes/ActiveRecord/Scoping/Named/ClassMethods.html#method-i-scope > The method is intended to return an ActiveRecord::Relation object, which > is composable with other scopes. Paginable.to_a_paginated_by_id returns an Array and is not appropriate as a scope. --- app/controllers/api/v1/admin/accounts_controller.rb | 2 +- app/controllers/api/v1/admin/reports_controller.rb | 2 +- app/controllers/api/v1/bookmarks_controller.rb | 2 +- app/controllers/api/v1/conversations_controller.rb | 2 +- app/controllers/api/v1/crypto/encrypted_messages_controller.rb | 2 +- app/controllers/api/v1/favourites_controller.rb | 2 +- app/controllers/api/v1/scheduled_statuses_controller.rb | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) (limited to 'app/controllers/api/v1') diff --git a/app/controllers/api/v1/admin/accounts_controller.rb b/app/controllers/api/v1/admin/accounts_controller.rb index c35ea5ab2..24c7fbef1 100644 --- a/app/controllers/api/v1/admin/accounts_controller.rb +++ b/app/controllers/api/v1/admin/accounts_controller.rb @@ -79,7 +79,7 @@ class Api::V1::Admin::AccountsController < Api::BaseController private def set_accounts - @accounts = filtered_accounts.order(id: :desc).includes(user: [:invite_request, :invite]).paginate_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) + @accounts = filtered_accounts.order(id: :desc).includes(user: [:invite_request, :invite]).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) end def set_account diff --git a/app/controllers/api/v1/admin/reports_controller.rb b/app/controllers/api/v1/admin/reports_controller.rb index 1d48d3160..c8f4cd8d8 100644 --- a/app/controllers/api/v1/admin/reports_controller.rb +++ b/app/controllers/api/v1/admin/reports_controller.rb @@ -63,7 +63,7 @@ class Api::V1::Admin::ReportsController < Api::BaseController private def set_reports - @reports = filtered_reports.order(id: :desc).with_accounts.paginate_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) + @reports = filtered_reports.order(id: :desc).with_accounts.to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) end def set_report diff --git a/app/controllers/api/v1/bookmarks_controller.rb b/app/controllers/api/v1/bookmarks_controller.rb index 5c72f4a1a..aa3fb88f0 100644 --- a/app/controllers/api/v1/bookmarks_controller.rb +++ b/app/controllers/api/v1/bookmarks_controller.rb @@ -21,7 +21,7 @@ class Api::V1::BookmarksController < Api::BaseController end def results - @_results ||= account_bookmarks.eager_load(:status).paginate_by_id( + @_results ||= account_bookmarks.eager_load(:status).to_a_paginated_by_id( limit_param(DEFAULT_STATUSES_LIMIT), params_slice(:max_id, :since_id, :min_id) ) diff --git a/app/controllers/api/v1/conversations_controller.rb b/app/controllers/api/v1/conversations_controller.rb index bc8013379..6c7583403 100644 --- a/app/controllers/api/v1/conversations_controller.rb +++ b/app/controllers/api/v1/conversations_controller.rb @@ -32,7 +32,7 @@ class Api::V1::ConversationsController < Api::BaseController def paginated_conversations AccountConversation.where(account: current_account) - .paginate_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) + .to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) end def insert_pagination_headers diff --git a/app/controllers/api/v1/crypto/encrypted_messages_controller.rb b/app/controllers/api/v1/crypto/encrypted_messages_controller.rb index c764915e5..68cf4384f 100644 --- a/app/controllers/api/v1/crypto/encrypted_messages_controller.rb +++ b/app/controllers/api/v1/crypto/encrypted_messages_controller.rb @@ -26,7 +26,7 @@ class Api::V1::Crypto::EncryptedMessagesController < Api::BaseController end def set_encrypted_messages - @encrypted_messages = @current_device.encrypted_messages.paginate_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) + @encrypted_messages = @current_device.encrypted_messages.to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) end def insert_pagination_headers diff --git a/app/controllers/api/v1/favourites_controller.rb b/app/controllers/api/v1/favourites_controller.rb index 71a707d2a..21836bc17 100644 --- a/app/controllers/api/v1/favourites_controller.rb +++ b/app/controllers/api/v1/favourites_controller.rb @@ -21,7 +21,7 @@ class Api::V1::FavouritesController < Api::BaseController end def results - @_results ||= account_favourites.eager_load(:status).paginate_by_id( + @_results ||= account_favourites.eager_load(:status).to_a_paginated_by_id( limit_param(DEFAULT_STATUSES_LIMIT), params_slice(:max_id, :since_id, :min_id) ) diff --git a/app/controllers/api/v1/scheduled_statuses_controller.rb b/app/controllers/api/v1/scheduled_statuses_controller.rb index 9950296f3..f90642a73 100644 --- a/app/controllers/api/v1/scheduled_statuses_controller.rb +++ b/app/controllers/api/v1/scheduled_statuses_controller.rb @@ -32,7 +32,7 @@ class Api::V1::ScheduledStatusesController < Api::BaseController private def set_statuses - @statuses = current_account.scheduled_statuses.paginate_by_id(limit_param(DEFAULT_STATUSES_LIMIT), params_slice(:max_id, :since_id, :min_id)) + @statuses = current_account.scheduled_statuses.to_a_paginated_by_id(limit_param(DEFAULT_STATUSES_LIMIT), params_slice(:max_id, :since_id, :min_id)) end def set_status -- cgit From 79305428a7c2bda311bc9d367a84acc28f569522 Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 1 Sep 2020 13:31:28 +0200 Subject: Add configuration option to filter replies in lists (#9205) * Add database support for list show-reply preferences * Add backend support to read and update list-specific show_replies settings * Add basic UI to set list replies setting * Add specs for list replies policy * Switch "cycling" reply policy link to a set of radio inputs * Capitalize replies_policy strings * Change radio button design to be consistent with that of the directory explorer --- app/controllers/api/v1/lists_controller.rb | 2 +- app/javascript/mastodon/actions/lists.js | 4 +- .../mastodon/features/list_timeline/index.js | 30 ++++++- app/javascript/styles/mastodon/components.scss | 4 + app/lib/feed_manager.rb | 3 +- app/models/list.rb | 13 +-- app/serializers/rest/list_serializer.rb | 2 +- .../20181127165847_add_show_replies_to_lists.rb | 23 +++++ db/schema.rb | 1 + spec/lib/feed_manager_spec.rb | 97 +++++++++++++++++++++- 10 files changed, 165 insertions(+), 14 deletions(-) create mode 100644 db/migrate/20181127165847_add_show_replies_to_lists.rb (limited to 'app/controllers/api/v1') diff --git a/app/controllers/api/v1/lists_controller.rb b/app/controllers/api/v1/lists_controller.rb index 054172bee..e5ac45fef 100644 --- a/app/controllers/api/v1/lists_controller.rb +++ b/app/controllers/api/v1/lists_controller.rb @@ -38,6 +38,6 @@ class Api::V1::ListsController < Api::BaseController end def list_params - params.permit(:title) + params.permit(:title, :replies_policy) end end diff --git a/app/javascript/mastodon/actions/lists.js b/app/javascript/mastodon/actions/lists.js index d736bacef..5ab922436 100644 --- a/app/javascript/mastodon/actions/lists.js +++ b/app/javascript/mastodon/actions/lists.js @@ -150,10 +150,10 @@ export const createListFail = error => ({ error, }); -export const updateList = (id, title, shouldReset) => (dispatch, getState) => { +export const updateList = (id, title, shouldReset, replies_policy) => (dispatch, getState) => { dispatch(updateListRequest(id)); - api(getState).put(`/api/v1/lists/${id}`, { title }).then(({ data }) => { + api(getState).put(`/api/v1/lists/${id}`, { title, replies_policy }).then(({ data }) => { dispatch(updateListSuccess(data)); if (shouldReset) { diff --git a/app/javascript/mastodon/features/list_timeline/index.js b/app/javascript/mastodon/features/list_timeline/index.js index f3205b2bf..a3be8fbea 100644 --- a/app/javascript/mastodon/features/list_timeline/index.js +++ b/app/javascript/mastodon/features/list_timeline/index.js @@ -10,15 +10,19 @@ import { addColumn, removeColumn, moveColumn } from '../../actions/columns'; import { FormattedMessage, defineMessages, injectIntl } from 'react-intl'; import { connectListStream } from '../../actions/streaming'; import { expandListTimeline } from '../../actions/timelines'; -import { fetchList, deleteList } from '../../actions/lists'; +import { fetchList, deleteList, updateList } from '../../actions/lists'; import { openModal } from '../../actions/modal'; import MissingIndicator from '../../components/missing_indicator'; import LoadingIndicator from '../../components/loading_indicator'; import Icon from 'mastodon/components/icon'; +import RadioButton from 'mastodon/components/radio_button'; const messages = defineMessages({ deleteMessage: { id: 'confirmations.delete_list.message', defaultMessage: 'Are you sure you want to permanently delete this list?' }, deleteConfirm: { id: 'confirmations.delete_list.confirm', defaultMessage: 'Delete' }, + all_replies: { id: 'lists.replies_policy.all_replies', defaultMessage: 'Any followed user' }, + no_replies: { id: 'lists.replies_policy.no_replies', defaultMessage: 'No one' }, + list_replies: { id: 'lists.replies_policy.list_replies', defaultMessage: 'Members of the list' }, }); const mapStateToProps = (state, props) => ({ @@ -131,11 +135,18 @@ class ListTimeline extends React.PureComponent { })); } + handleRepliesPolicyChange = ({ target }) => { + const { dispatch } = this.props; + const { id } = this.props.params; + dispatch(updateList(id, undefined, false, target.value)); + } + render () { - const { shouldUpdateScroll, hasUnread, columnId, multiColumn, list } = this.props; + const { shouldUpdateScroll, hasUnread, columnId, multiColumn, list, intl } = this.props; const { id } = this.props.params; const pinned = !!columnId; const title = list ? list.get('title') : id; + const replies_policy = list ? list.get('replies_policy') : undefined; if (typeof list === 'undefined') { return ( @@ -166,7 +177,7 @@ class ListTimeline extends React.PureComponent { pinned={pinned} multiColumn={multiColumn} > -
+
@@ -175,6 +186,19 @@ class ListTimeline extends React.PureComponent {
+ + { replies_policy !== undefined && ( +
+ + + +
+ { ['no_replies', 'list_replies', 'all_replies'].map(policy => ( + + ))} +
+
+ )}