Skip to content
Snippets Groups Projects
Verified Commit 183ee8ba authored by Heinrich Lee Yu's avatar Heinrich Lee Yu
Browse files

Remove paginated_issue_discussions FF

Enables batch loading of issue discussions

Changelog: performance
parent cb549e15
No related branches found
No related tags found
1 merge request!76089Remove paginated_issue_discussions FF
......@@ -10,8 +10,8 @@ export const OPENED = 'opened';
export const REOPENED = 'reopened';
export const CLOSED = 'closed';
export const MERGED = 'merged';
export const ISSUE_NOTEABLE_TYPE = 'issue';
export const EPIC_NOTEABLE_TYPE = 'epic';
export const ISSUE_NOTEABLE_TYPE = 'Issue';
export const EPIC_NOTEABLE_TYPE = 'Epic';
export const MERGE_REQUEST_NOTEABLE_TYPE = 'MergeRequest';
export const UNRESOLVE_NOTE_METHOD_NAME = 'delete';
export const RESOLVE_NOTE_METHOD_NAME = 'post';
......
......@@ -83,14 +83,17 @@ export const setExpandDiscussions = ({ commit }, { discussionIds, expanded }) =>
commit(types.SET_EXPAND_DISCUSSIONS, { discussionIds, expanded });
};
export const fetchDiscussions = ({ commit, dispatch }, { path, filter, persistFilter }) => {
export const fetchDiscussions = (
{ commit, dispatch, getters },
{ path, filter, persistFilter },
) => {
const config =
filter !== undefined
? { params: { notes_filter: filter, persist_filter: persistFilter } }
: null;
if (
window.gon?.features?.paginatedIssueDiscussions ||
getters.noteableType === constants.ISSUE_NOTEABLE_TYPE ||
window.gon?.features?.paginatedMrDiscussions
) {
return dispatch('fetchDiscussionsBatch', { path, config, perPage: 20 });
......@@ -114,7 +117,7 @@ export const fetchDiscussionsBatch = ({ commit, dispatch }, { path, config, curs
return axios.get(path, { params }).then(({ data, headers }) => {
commit(types.ADD_OR_UPDATE_DISCUSSIONS, data);
if (headers['x-next-page-cursor']) {
if (headers && headers['x-next-page-cursor']) {
const nextConfig = { ...config };
if (config?.params?.persist_filter) {
......
......@@ -184,7 +184,6 @@ def discussions
def paginated_discussions
return if params[:per_page].blank?
return if issuable.instance_of?(Issue) && Feature.disabled?(:paginated_issue_discussions, project)
return if issuable.instance_of?(MergeRequest) && Feature.disabled?(:paginated_mr_discussions, project)
strong_memoize(:paginated_discussions) do
......
......@@ -48,7 +48,6 @@ class Projects::IssuesController < Projects::ApplicationController
before_action only: :show do
push_frontend_feature_flag(:confidential_notes, project&.group)
push_frontend_feature_flag(:issue_assignees_widget, project)
push_frontend_feature_flag(:paginated_issue_discussions, project)
push_frontend_feature_flag(:realtime_labels, project)
push_force_frontend_feature_flag(:work_items, project&.work_items_feature_flag_enabled?)
push_frontend_feature_flag(:work_items_mvc_2)
......
- add_page_startup_api_call Feature.enabled?(:paginated_issue_discussions, @project) ? discussions_path(@issue, per_page: 20) : discussions_path(@issue)
- add_page_startup_api_call discussions_path(@issue, per_page: 20)
- @gfm_form = true
......
---
name: paginated_issue_discussions
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69933
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345351
milestone: '14.5'
type: development
group: group::project management
default_enabled: false
......@@ -217,8 +217,7 @@ describe('note_app', () => {
});
afterEach(() => {
waitForDiscussionsRequest();
resetHTMLFixture();
return waitForDiscussionsRequest().then(() => resetHTMLFixture());
});
it('renders skeleton notes', () => {
......@@ -471,7 +470,7 @@ describe('note_app', () => {
wrapper = shallowMount(NotesApp, { propsData, store: createStore() });
await waitForPromises();
expect(axiosMock.history.get[0].params).toBeUndefined();
expect(axiosMock.history.get[0].params).toEqual({ per_page: 20 });
});
});
......@@ -496,14 +495,14 @@ describe('note_app', () => {
wrapper = mountWithNotesFilter(undefined);
await waitForPromises();
expect(axiosMock.history.get[0].params).toBeUndefined();
expect(axiosMock.history.get[0].params).toEqual({ per_page: 20 });
});
it('does not include extra query params when filter is already set to default', async () => {
wrapper = mountWithNotesFilter(constants.DISCUSSION_FILTERS_DEFAULT_VALUE);
await waitForPromises();
expect(axiosMock.history.get[0].params).toBeUndefined();
expect(axiosMock.history.get[0].params).toEqual({ per_page: 20 });
});
it('includes extra query params when filter is not set to default', async () => {
......@@ -512,6 +511,7 @@ describe('note_app', () => {
expect(axiosMock.history.get[0].params).toEqual({
notes_filter: constants.DISCUSSION_FILTERS_DEFAULT_VALUE,
per_page: 20,
persist_filter: false,
});
});
......
......@@ -1351,7 +1351,7 @@ describe('Actions Notes Store', () => {
return testAction(
actions.fetchDiscussions,
{},
null,
{ noteableType: notesConstants.MERGE_REQUEST_NOTEABLE_TYPE },
[
{ type: mutationTypes.ADD_OR_UPDATE_DISCUSSIONS, payload: { discussion } },
{ type: mutationTypes.SET_FETCHING_DISCUSSIONS, payload: false },
......@@ -1360,13 +1360,11 @@ describe('Actions Notes Store', () => {
);
});
it('dispatches `fetchDiscussionsBatch` action if `paginatedIssueDiscussions` feature flag is enabled', () => {
window.gon = { features: { paginatedIssueDiscussions: true } };
it('dispatches `fetchDiscussionsBatch` action if noteable is an Issue', () => {
return testAction(
actions.fetchDiscussions,
{ path: 'test-path', filter: 'test-filter', persistFilter: 'test-persist-filter' },
null,
{ noteableType: notesConstants.ISSUE_NOTEABLE_TYPE },
[],
[
{
......@@ -1389,7 +1387,7 @@ describe('Actions Notes Store', () => {
return testAction(
actions.fetchDiscussions,
{ path: 'test-path', filter: 'test-filter', persistFilter: 'test-persist-filter' },
null,
{ noteableType: notesConstants.MERGE_REQUEST_NOTEABLE_TYPE },
[],
[
{
......
......@@ -50,22 +50,6 @@ def get_discussions(**params)
a_hash_including('id' => discussion_2.id.to_s)
])
end
context 'when paginated_issue_discussions is disabled' do
before do
stub_feature_flags(paginated_issue_discussions: false)
end
it 'returns all discussions and ignores per_page param' do
get_discussions(per_page: 2)
discussions = Gitlab::Json.parse(response.body)
notes = discussions.flat_map { |d| d['notes'] }
expect(discussions.count).to eq(4)
expect(notes.count).to eq(5)
end
end
end
end
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment