ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Re-Add upload button to Discussion redesign 

fixes VICE-3079

flag=react_discussions_post
flag=isolated_view
flag=split_screen_view

Test PLan:
1) Backend tests pass;
2) ATTACHMENTS_FOLDER_ID env set;
3) Should be able to Add, Update, Delete attachment
 for a discussion entry.
-- try for splitscreen, isolated view, and
 normal( w/o either split or isolated)

Change-Id: I3bb5ba0326a465b481764c94b507f09332ab49d4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/304852
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Omar Soto-Fortuño <omar.soto@instructure.com>
Product-Review: Omar Soto-Fortuño <omar.soto@instructure.com>
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
This commit is contained in:
Chawn Neal 2022-11-07 23:13:31 -05:00
parent 30cf11164d
commit d76b759a46
16 changed files with 228 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/controllers/discussion_topics_controller.rb
View File

@ -793,7 +793,8 @@ class DiscussionTopicsController < ApplicationController
# GRADED_RUBRICS_URL must be within DISCUSSION to avoid page error
DISCUSSION: {
GRADED_RUBRICS_URL: (@topic.assignment ? context_url(@topic.assignment.context, :context_assignment_rubric_url, @topic.assignment.id) : nil),
CONTEXT_RUBRICS_URL: can_do(@topic.assignment, @current_user, :update) ? context_url(@topic.assignment.context, :context_rubrics_url) : ""
CONTEXT_RUBRICS_URL: can_do(@topic.assignment, @current_user, :update) ? context_url(@topic.assignment.context, :context_rubrics_url) : "",
ATTACHMENTS_FOLDER_ID: (@topic.for_assignment? && !@current_user.nil?) ? @current_user.submissions_folder(@context).id : Folder.unfiled_folder(@context).id
},
apollo_caching: @current_user &&
Account.site_admin.feature_enabled?(:apollo_caching),

40
spec/controllers/discussion_topics_controller_spec.rb
View File

@ -551,6 +551,46 @@ describe DiscussionTopicsController do
user_session(user)
end
it "sets ATTACHMENTS_FOLDER_ID" do
subject
expect(discussion).not_to be_for_assignment
expect(assigns[:js_env][:DISCUSSION][:ATTACHMENTS_FOLDER_ID]).to eq Folder.unfiled_folder(discussion.course).id.to_s
end
context "no current user" do
it "public course sets ATTACHMENTS_FOLDER_ID" do
Account.default.enable_feature! :react_discussions_post
# in the controller 'can_read_and_visible' must be true, which is a complex flow to simulate
allow_any_instance_of(DiscussionTopic).to receive(:grants_right?).and_return(true)
allow_any_instance_of(DiscussionTopic).to receive(:visible_for?).and_return(true)
course.update(is_public: true)
discussion.assignment = course.assignments.build(submission_types: "discussion_topic", title: discussion.title)
discussion.assignment.infer_times
discussion.assignment.saved_by = :discussion_topic
discussion.save
remove_user_session
subject
expect(discussion).to be_for_assignment
expect(assigns[:js_env][:DISCUSSION][:ATTACHMENTS_FOLDER_ID]).to eq Folder.unfiled_folder(discussion.course).id.to_s
end
end
context "for_assignment" do
it "sets ATTACHMENTS_FOLDER_ID" do
discussion.assignment = course.assignments.build(submission_types: "discussion_topic", title: discussion.title)
discussion.assignment.infer_times
discussion.assignment.saved_by = :discussion_topic
discussion.save
subject
expect(discussion).to be_for_assignment
expect(assigns[:js_env][:DISCUSSION][:ATTACHMENTS_FOLDER_ID]).to eq user.submissions_folder(discussion.course).id.to_s
end
end
it "sets @page_title to Topic: @topic.title" do
subject
expect(assigns(:page_title)).to eq "Topic: #{discussion.title}"

6
ui/features/discussion_topics_post/graphql/Mocks.js
View File

@ -353,7 +353,8 @@ export const updateDiscussionEntryParticipantMock = ({
export const updateDiscussionEntryMock = ({
discussionEntryId = 'DiscussionEntry-default-mock',
message = '<p>This is the parent reply</p>',
removeAttachment = !'7',
fileId = '7',
removeAttachment = !fileId,
} = {}) => [
{
request: {
@ -361,6 +362,7 @@ export const updateDiscussionEntryMock = ({
variables: {
discussionEntryId,
message,
...(fileId !== null && {fileId}),
removeAttachment,
},
},
@ -413,6 +415,7 @@ export const createDiscussionEntryMock = ({
discussionTopicId = 'Discussion-default-mock',
message = '',
replyFromEntryId = null,
fileId = null,
includeReplyPreview = null,
isAnonymousAuthor = false,
courseID = '1',
@ -425,6 +428,7 @@ export const createDiscussionEntryMock = ({
message,
isAnonymousAuthor,
...(replyFromEntryId !== null && {replyFromEntryId}),
...(fileId !== null && {fileId}),
...(includeReplyPreview !== null && {includeReplyPreview}),
...(courseID !== null && {courseID}),
},

4
ui/features/discussion_topics_post/graphql/Mutations.js
View File

@ -132,6 +132,7 @@ export const CREATE_DISCUSSION_ENTRY = gql`
$discussionTopicId: ID!
$message: String!
$replyFromEntryId: ID
$fileId: ID
$includeReplyPreview: Boolean
$isAnonymousAuthor: Boolean
$courseID: String
@ -141,6 +142,7 @@ export const CREATE_DISCUSSION_ENTRY = gql`
discussionTopicId: $discussionTopicId
message: $message
parentEntryId: $replyFromEntryId
fileId: $fileId
includeReplyPreview: $includeReplyPreview
isAnonymousAuthor: $isAnonymousAuthor
}
@ -172,12 +174,14 @@ export const UPDATE_DISCUSSION_ENTRY = gql`
mutation UpdateDiscussionEntry(
$discussionEntryId: ID!
$message: String
$fileId: ID
$removeAttachment: Boolean
) {
updateDiscussionEntry(
input: {
discussionEntryId: $discussionEntryId
message: $message
fileId: $fileId
removeAttachment: $removeAttachment
}
) {

3
ui/features/discussion_topics_post/react/DiscussionTopicManager.js
View File

@ -306,11 +306,12 @@ const DiscussionTopicManager = props => {
<DiscussionTopicContainer
updateDraftCache={updateDraftCache}
discussionTopic={discussionTopicQuery.data.legacyNode}
createDiscussionEntry={(message, isAnonymousAuthor) => {
createDiscussionEntry={(message, fileId, isAnonymousAuthor) => {
createDiscussionEntry({
variables: {
discussionTopicId: ENV.discussion_topic_id,
message,
fileId,
courseID: ENV.course_id,
isAnonymousAuthor,
},

3
ui/features/discussion_topics_post/react/__tests__/DiscussionsAttachment.test.js
View File

@ -33,7 +33,7 @@ jest.mock('../utils', () => ({
responsiveQuerySizes: jest.fn(),
}))
describe('DiscussionThreadAttachment', () => {
describe('DiscussionsAttachment', () => {
const onFailureStub = jest.fn()
const onSuccessStub = jest.fn()
const openMock = jest.fn()
@ -90,6 +90,7 @@ describe('DiscussionThreadAttachment', () => {
updateDiscussionEntryMock({
discussionEntryId: 'DiscussionEntry-default-mock',
message: '<p>This is the parent reply</p>',
fileId: null,
removeAttachment: true,
})
)

57
ui/features/discussion_topics_post/react/components/AttachmentDisplay/AttachmentDisplay.js
View File

@ -17,17 +17,55 @@
*/
import PropTypes from 'prop-types'
import React from 'react'
import React, {useContext} from 'react'
import {AttachmentButton} from './AttachmentButton'
import {AlertManagerContext} from '@canvas/alerts/react/AlertManager'
import {Responsive} from '@instructure/ui-responsive'
import {responsiveQuerySizes} from '../../utils'
import {UploadButton} from './UploadButton'
import {uploadFiles} from '@canvas/upload-file'
import {useScope as useI18nScope} from '@canvas/i18n'
const I18n = useI18nScope('discussion_topics_post')
export function AttachmentDisplay(props) {
const {setOnFailure, setOnSuccess} = useContext(AlertManagerContext)
const removeAttachment = () => {
props.setAttachment(null)
}
const fileUploadUrl = attachmentFolderId => {
return `/api/v1/folders/${attachmentFolderId}/files`
}
const addAttachment = async e => {
const files = Array.from(e.currentTarget?.files)
if (files.length !== 1) {
setOnFailure(I18n.t('Error adding file to discussion message'))
}
props.setAttachmentToUpload(true)
setOnSuccess(I18n.t('Uploading file'))
try {
const newFiles = await uploadFiles(
files,
fileUploadUrl(ENV.DISCUSSION?.ATTACHMENTS_FOLDER_ID)
)
const newFile = {
_id: newFiles[0].id,
url: newFiles[0].url,
displayName: newFiles[0].display_name,
}
props.setAttachment(newFile)
} catch (err) {
setOnFailure(I18n.t('Error uploading file'))
} finally {
props.setAttachmentToUpload(false)
}
}
return (
<Responsive
match="media"
@ -43,8 +81,13 @@ export function AttachmentDisplay(props) {
},
}}
render={(_responsiveProps, _matches) =>
props.attachment?._id && (
props.attachment?._id ? (
<AttachmentButton attachment={props.attachment} onDeleteItem={removeAttachment} />
) : (
<UploadButton
attachmentToUpload={props.attachmentToUpload}
onAttachmentUpload={addAttachment}
/>
)
}
/>
@ -61,6 +104,14 @@ AttachmentDisplay.propTypes = {
* Used to set the attachments prop, if no attachment is set
*/
setAttachment: PropTypes.func.isRequired,
/**
* Used to set the setAttachmentsToUpload prop, allows for returning loading state
*/
setAttachmentToUpload: PropTypes.func.isRequired,
/**
* toggles loading state
*/
attachmentToUpload: PropTypes.bool,
}
export default AttachmentDisplay

72
ui/features/discussion_topics_post/react/components/AttachmentDisplay/UploadButton.js Normal file
View File

@ -0,0 +1,72 @@
/*
* Copyright (C) 2022 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import PropTypes from 'prop-types'
import React from 'react'
import {CondensedButton} from '@instructure/ui-buttons'
import {IconPaperclipLine} from '@instructure/ui-icons'
import {Spinner} from '@instructure/ui-spinner'
import {Text} from '@instructure/ui-text'
import {useScope as useI18nScope} from '@canvas/i18n'
const I18n = useI18nScope('discussion_topics_post')
export const UploadButton = ({...props}) => {
let attachmentInput = null
const handleAttachmentClick = () => attachmentInput?.click()
return props.attachmentToUpload ? (
<>
<Spinner
renderTitle={I18n.t('Uploading file in progress')}
margin="0 0 0 small"
size="x-small"
/>
</>
) : (
<>
<CondensedButton
color="primary"
renderIcon={<IconPaperclipLine size="small" />}
onClick={handleAttachmentClick}
data-testid="attach-btn"
>
<Text weight="bold">{I18n.t('Attach')}</Text>
</CondensedButton>
<input
data-testid="attachment-input"
ref={input => (attachmentInput = input)}
type="file"
style={{display: 'none'}}
aria-hidden={true}
onChange={props.onAttachmentUpload}
/>
</>
)
}
UploadButton.propTypes = {
/**
* function that performs on the file after button click, then upload file, upload
*/
onAttachmentUpload: PropTypes.func.isRequired,
/**
* toggles loading state
*/
attachmentToUpload: PropTypes.bool,
}

17
ui/features/discussion_topics_post/react/components/AttachmentDisplay/__tests__/AttachmentDisplay.test.js
View File

@ -21,10 +21,23 @@ import {render} from '@testing-library/react'
import {AttachmentDisplay} from '../AttachmentDisplay'
const setup = props => {
return render(<AttachmentDisplay setAttachment={() => {}} {...props} />)
return render(
<AttachmentDisplay setAttachment={() => {}} setAttachmentToUpload={() => {}} {...props} />
)
}
describe('AttachmentDisplay', () => {
it('displays AttachButton when there is no attachment', () => {
const {queryByText} = setup()
expect(queryByText('Attach')).toBeTruthy()
})
it('only allows one attachment at a time', () => {
const {queryByTestId} = setup()
expect(queryByTestId('attachment-input')).toHaveAttribute('type', 'file')
expect(queryByTestId('attachment-input')).not.toHaveAttribute('multiple')
})
it('displays AttachmentButton when there is an attachment', () => {
const {queryByText} = setup({
attachment: {
@ -34,6 +47,7 @@ describe('AttachmentDisplay', () => {
},
})
expect(queryByText('Attach')).toBeFalsy()
expect(queryByText('file_name.file')).toBeTruthy()
})
@ -46,6 +60,7 @@ describe('AttachmentDisplay', () => {
},
})
expect(queryByText('Attach')).toBeFalsy()
expect(queryByText('Fundamentals of Differential E...')).toBeTruthy()
})
})

16
ui/features/discussion_topics_post/react/components/DiscussionEdit/DiscussionEdit.js
View File

@ -52,6 +52,7 @@ export const DiscussionEdit = props => {
)
const [attachment, setAttachment] = useState(null)
const [attachmentToUpload, setAttachmentToUpload] = useState(false)
const rceMentionsIsEnabled = () => {
return !!ENV.rce_mentions_in_discussions
@ -226,6 +227,7 @@ export const DiscussionEdit = props => {
color="primary"
data-testid="DiscussionEdit-submit"
key="rce-reply-button"
interaction={attachmentToUpload ? 'disabled' : 'enabled'}
>
<Text size="medium">{props.isEdit ? I18n.t('Save') : I18n.t('Reply')}</Text>
</Button>
@ -235,7 +237,12 @@ export const DiscussionEdit = props => {
return matches.includes('mobile') ? (
<View as="div" padding={undefined} key="mobileButtons">
<View as={responsiveProps.viewAs} padding={responsiveProps.paddingAttachment}>
<AttachmentDisplay attachment={attachment} setAttachment={setAttachment} />
<AttachmentDisplay
attachment={attachment}
setAttachment={setAttachment}
setAttachmentToUpload={setAttachmentToUpload}
attachmentToUpload={attachmentToUpload}
/>
</View>
{rceButtons.reverse()}
</View>
@ -243,7 +250,12 @@ export const DiscussionEdit = props => {
<Flex key="nonMobileButtons">
<Flex.Item shouldGrow={true} textAlign="start">
<View as={responsiveProps.viewAs} padding={responsiveProps.paddingAttachment}>
<AttachmentDisplay attachment={attachment} setAttachment={setAttachment} />
<AttachmentDisplay
attachment={attachment}
setAttachment={setAttachment}
setAttachmentToUpload={setAttachmentToUpload}
attachmentToUpload={attachmentToUpload}
/>
</View>
</Flex.Item>
{ENV.draft_discussions && (

8
ui/features/discussion_topics_post/react/containers/DiscussionThreadContainer/DiscussionThreadContainer.js
View File

@ -288,6 +288,7 @@ export const DiscussionThreadContainer = props => {
variables: {
discussionEntryId: props.discussionEntry._id,
message,
fileId,
removeAttachment: !fileId,
},
})
@ -325,7 +326,7 @@ export const DiscussionThreadContainer = props => {
}
}, [threadRefCurrent, props.discussionEntry.entryParticipant.read, props])
const onReplySubmit = (message, isAnonymousAuthor, includeReplyPreview) => {
const onReplySubmit = (message, fileId, isAnonymousAuthor, includeReplyPreview) => {
createDiscussionEntry({
variables: {
discussionTopicId: ENV.discussion_topic_id,
@ -334,6 +335,7 @@ export const DiscussionThreadContainer = props => {
props.discussionEntry.rootEntryId !== props.discussionEntry.parentId
? props.discussionEntry.parentId
: props.discussionEntry._id,
fileId,
isAnonymousAuthor,
includeReplyPreview,
message,
@ -490,8 +492,8 @@ export const DiscussionThreadContainer = props => {
<DiscussionEdit
discussionAnonymousState={props.discussionTopic?.anonymousState}
canReplyAnonymously={props.discussionTopic?.canReplyAnonymously}
onSubmit={(message, includeReplyPreview, _fileId, anonymousAuthorState) => {
onReplySubmit(message, anonymousAuthorState, includeReplyPreview)
onSubmit={(message, includeReplyPreview, fileId, anonymousAuthorState) => {
onReplySubmit(message, fileId, anonymousAuthorState, includeReplyPreview)
}}
onCancel={() => setEditorExpanded(false)}
quotedEntry={buildQuotedReply([props.discussionEntry], replyFromId)}

4
ui/features/discussion_topics_post/react/containers/DiscussionTopicContainer/DiscussionTopicContainer.js
View File

@ -496,11 +496,11 @@ export const DiscussionTopicContainer = ({createDiscussionEntry, ...props}) => {
onSubmit={(
message,
_includeReplyPreview,
_fileId,
fileId,
anonymousAuthorState
) => {
if (createDiscussionEntry) {
createDiscussionEntry(message, anonymousAuthorState)
createDiscussionEntry(message, fileId, anonymousAuthorState)
setExpandedReply(false)
props.onDiscussionReplyPost()
}

1
ui/features/discussion_topics_post/react/containers/IsolatedThreadsContainer/IsolatedThreadsContainer.js
View File

@ -263,6 +263,7 @@ const IsolatedThreadContainer = props => {
variables: {
discussionEntryId: props.discussionEntry._id,
message,
fileId,
removeAttachment: !fileId,
},
})

6
ui/features/discussion_topics_post/react/containers/IsolatedViewContainer/IsolatedViewContainer.js
View File

@ -197,13 +197,14 @@ export const IsolatedViewContainer = props => {
window.open(getSpeedGraderUrl(discussionEntry.author._id), '_blank')
}
const onReplySubmit = (message, includeReplyPreview, replyId, isAnonymousAuthor) => {
const onReplySubmit = (message, fileId, includeReplyPreview, replyId, isAnonymousAuthor) => {
createDiscussionEntry({
variables: {
discussionTopicId: props.discussionTopic._id,
replyFromEntryId: replyId,
isAnonymousAuthor,
message,
fileId,
includeReplyPreview,
courseID: ENV.course_id,
},
@ -420,9 +421,10 @@ export const IsolatedViewContainer = props => {
<DiscussionEdit
discussionAnonymousState={props.discussionTopic?.anonymousState}
canReplyAnonymously={props.discussionTopic?.canReplyAnonymously}
onSubmit={(message, includeReplyPreview, _fileId, anonymousAuthorState) => {
onSubmit={(message, includeReplyPreview, fileId, anonymousAuthorState) => {
onReplySubmit(
message,
fileId,
includeReplyPreview,
props.replyFromId,
anonymousAuthorState

1
ui/features/discussion_topics_post/react/containers/SplitScreenThreadsContainer/SplitScreenThreadsContainer.js
View File

@ -263,6 +263,7 @@ const SplitScreenThreadContainer = props => {
variables: {
discussionEntryId: props.discussionEntry._id,
message,
fileId,
removeAttachment: !fileId,
},
})

6
ui/features/discussion_topics_post/react/containers/SplitScreenViewContainer/SplitScreenViewContainer.js
View File

@ -198,13 +198,14 @@ export const SplitScreenViewContainer = props => {
window.open(getSpeedGraderUrl(discussionEntry.author._id), '_blank')
}
const onReplySubmit = (message, includeReplyPreview, replyId, isAnonymousAuthor) => {
const onReplySubmit = (message, fileId, includeReplyPreview, replyId, isAnonymousAuthor) => {
createDiscussionEntry({
variables: {
discussionTopicId: props.discussionTopic._id,
replyFromEntryId: replyId,
isAnonymousAuthor,
message,
fileId,
includeReplyPreview,
courseID: ENV.course_id,
},
@ -421,9 +422,10 @@ export const SplitScreenViewContainer = props => {
<DiscussionEdit
discussionAnonymousState={props.discussionTopic?.anonymousState}
canReplyAnonymously={props.discussionTopic?.canReplyAnonymously}
onSubmit={(message, includeReplyPreview, _fileId, anonymousAuthorState) => {
onSubmit={(message, includeReplyPreview, fileId, anonymousAuthorState) => {
onReplySubmit(
message,
fileId,
includeReplyPreview,
props.replyFromId,
anonymousAuthorState