Add setting to open todos in new tab

If the user feature is enabled, teacher todos on the k5 and classic
dashboards open in a new tab (and otherwise they open in the same tab).

Note: this does not affect student todos

closes LS-2667
flag = open_todos_in_new_tab

Test plan:
 - Enable the feature (its on by default)
 - Visit the k5 dashboard and the classic dashboard as a teacher
 - Click on a todo item (if none exist, have a student submit an
   assignment that needs grading)
 - Expect it to open in a new tab
 - Disable the feature
 - In both locations, expect clicking the todo to open in the same
   tab

Change-Id: Ib49a0a6aff50c17a135a2859bf4e4fe802645d06
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279308
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ed Schiebel <eschiebel@instructure.com>
QA-Review: Ed Schiebel <eschiebel@instructure.com>
Product-Review: Jackson Howe <jackson.howe@instructure.com>

app/controllers/users_controller.rb

@@ -537,7 +537,8 @@ class UsersController < ApplicationController
                CAN_ADD_OBSERVEE: @current_user
                           .profile
                           .tabs_available(@current_user, :root_account => @domain_root_account)
-                          .any? { |t| t[:id] == UserProfile::TAB_OBSERVEES }
+                          .any? { |t| t[:id] == UserProfile::TAB_OBSERVEES },
+               OPEN_TEACHER_TODOS_IN_NEW_TAB: @current_user.feature_enabled?(:open_todos_in_new_tab)
              })
 
       # prefetch dashboard cards with the right observer url param

app/views/courses/_to_do_list.html.erb

@@ -41,7 +41,7 @@ show_legacy_todo_list ||= false
           <a
             class="item"
             href="<%= assignment.gradebook_path %>"
-            target="_blank"
+            <% if @current_user.feature_enabled?(:open_todos_in_new_tab) %>target="_blank"<% end %>
             data-track-category="dashboard"
             data-track-label="todo needs grading"
           >

config/feature_flags/00_standard.yml

@@ -439,3 +439,8 @@ gradebook_show_first_last_names:
   display_name: Separate columns for first/last names in Gradebook
   description: Show account setting to allow viewing and exporting of student first and last names
     in separate columns in the gradebook.
+open_todos_in_new_tab:
+  applies_to: User
+  state: allowed_on
+  display_name: Open to-do items in a new tab
+  description: When enabled, this setting automatically opens teacher to-do items in a new tab.

spec/selenium/dashboard/dashboard_teacher_spec.rb

@@ -265,6 +265,35 @@ describe "dashboard" do
       expect(todo_list).to include_text(quiz_title)
     end
 
+    context "todo link" do
+      before do
+        assignment = assignment_model({ :submission_types => 'online_text_entry', :course => @course })
+        student = user_with_pseudonym(:active_user => true, :username => 'student@example.com', :password => 'qwertyuiop')
+        @course.enroll_user(student, "StudentEnrollment", :enrollment_state => 'active')
+        assignment.reload
+        assignment.submit_homework(student, { :submission_type => 'online_text_entry', :body => 'ABC' })
+        assignment.reload
+      end
+
+      it "opens in a new tab if open_todos_in_new_tab is enabled" do
+        @teacher.enable_feature!(:open_todos_in_new_tab)
+        enable_cache do
+          get "/"
+          link = f('.to-do-list > li > a')
+          expect(link.attribute('target')).to eq('_blank')
+        end
+      end
+
+      it "opens in same tab if open_todos_in_new_tab is disabled" do
+        @teacher.disable_feature!(:open_todos_in_new_tab)
+        enable_cache do
+          get "/"
+          link = f('.to-do-list > li > a')
+          expect(link.attribute('target')).to be_empty
+        end
+      end
+    end
+
     context "course menu customization" do
       it "always has a link to the courses page (with customizations)", priority: "1" do
         course_with_teacher({ :user => @user, :active_course => true, :active_enrollment => true })

ui/features/k5_dashboard/index.js

@@ -43,6 +43,7 @@ ready(() => {
         parentSupportEnabled={ENV.FEATURES?.k5_parent_support}
         observerList={ENV.OBSERVER_LIST}
         canAddObservee={ENV.CAN_ADD_OBSERVEE}
+        openTodosInNewTab={ENV.OPEN_TEACHER_TODOS_IN_NEW_TAB}
       />,
       dashboardContainer
     )

ui/features/k5_dashboard/react/K5Dashboard.js

@@ -141,7 +141,8 @@ export const K5Dashboard = ({
   selectedContextsLimit,
   parentSupportEnabled,
   observerList,
-  canAddObservee
+  canAddObservee,
+  openTodosInNewTab
 }) => {
   const initialObservedId = observerList.find(o => o.id === savedObservedId(currentUser.id))
     ? savedObservedId(currentUser.id)
@@ -372,7 +373,11 @@ export const K5Dashboard = ({
               />
             )}
             {currentUserRoles.includes('teacher') && (
-              <TodosPage timeZone={timeZone} visible={currentTab === TAB_IDS.TODO} />
+              <TodosPage
+                timeZone={timeZone}
+                openTodosInNewTab={openTodosInNewTab}
+                visible={currentTab === TAB_IDS.TODO}
+              />
             )}
           </K5DashboardContext.Provider>
         </Flex.Item>
@@ -421,7 +426,8 @@ K5Dashboard.propTypes = {
   selectedContextsLimit: PropTypes.number.isRequired,
   parentSupportEnabled: PropTypes.bool.isRequired,
   observerList: ObserverListShape.isRequired,
-  canAddObservee: PropTypes.bool.isRequired
+  canAddObservee: PropTypes.bool.isRequired,
+  openTodosInNewTab: PropTypes.bool.isRequired
 }
 
 const WrappedK5Dashboard = connect(mapStateToProps)(responsiviser()(K5Dashboard))

ui/features/k5_dashboard/react/Todo.js

@@ -37,7 +37,15 @@ import tz from '@canvas/timezone'
 export const getBaseDueAt = ({all_dates}) =>
   (all_dates.filter(d => d.base)[0] || all_dates[0])?.due_at
 
-const Todo = ({assignment, context_name, html_url, ignore, needs_grading_count, timeZone}) => {
+const Todo = ({
+  assignment,
+  context_name,
+  html_url,
+  ignore,
+  needs_grading_count,
+  timeZone,
+  openInNewTab
+}) => {
   const [ignored, setIgnored] = useState(false)
 
   // Only assignments are supported (ungraded_quizzes are not)
@@ -87,7 +95,7 @@ const Todo = ({assignment, context_name, html_url, ignore, needs_grading_count,
       <Flex as="div" direction="column" margin="0 small 0 0" width="27rem">
         <Link
           href={html_url}
-          target="_blank"
+          target={openInNewTab ? '_blank' : undefined}
           isWithinText={false}
           theme={{
             fontWeight: '700'
@@ -146,7 +154,8 @@ Todo.propTypes = {
   html_url: PropTypes.string.isRequired,
   ignore: PropTypes.string.isRequired,
   needs_grading_count: PropTypes.number,
-  timeZone: PropTypes.string.isRequired
+  timeZone: PropTypes.string.isRequired,
+  openInNewTab: PropTypes.bool.isRequired
 }
 
 export default Todo

ui/features/k5_dashboard/react/TodosPage.js

@@ -39,7 +39,7 @@ export const sortTodos = (t1, t2) => {
   return d1.localeCompare(d2)
 }
 
-export const TodosPage = ({timeZone, visible}) => {
+export const TodosPage = ({timeZone, visible, openTodosInNewTab}) => {
   const [loading, setLoading] = useState(true)
   const [todos, setTodos] = useState(null)
 
@@ -100,7 +100,12 @@ export const TodosPage = ({timeZone, visible}) => {
       >
         {todos?.length > 0 ? (
           todos.map(todo => (
-            <Todo key={`todo-assignment-${todo.assignment?.id}`} timeZone={timeZone} {...todo} />
+            <Todo
+              key={`todo-assignment-${todo.assignment?.id}`}
+              timeZone={timeZone}
+              openInNewTab={openTodosInNewTab}
+              {...todo}
+            />
           ))
         ) : (
           <EmptyTodos />
@@ -112,7 +117,8 @@ export const TodosPage = ({timeZone, visible}) => {
 
 TodosPage.propTypes = {
   timeZone: PropTypes.string.isRequired,
-  visible: PropTypes.bool.isRequired
+  visible: PropTypes.bool.isRequired,
+  openTodosInNewTab: PropTypes.bool.isRequired
 }
 
 export default TodosPage

ui/features/k5_dashboard/react/__tests__/K5Dashboard.test.js

@@ -197,7 +197,8 @@ const defaultProps = {
   selectedContextsLimit: 2,
   parentSupportEnabled: false,
   canAddObservee: false,
-  observerList: MOCK_OBSERVER_LIST
+  observerList: MOCK_OBSERVER_LIST,
+  openTodosInNewTab: true
 }
 
 beforeAll(() => {

ui/features/k5_dashboard/react/__tests__/Todo.test.js

@@ -32,7 +32,8 @@ const timeZone = 'Europe/Dublin'
 
 const defaultProps = {
   ...MOCK_TODOS[1],
-  timeZone
+  timeZone,
+  openInNewTab: true
 }
 
 beforeEach(() => {
@@ -163,4 +164,16 @@ describe('Todo', () => {
 
     expect((await findAllByText('Failed to ignore assignment'))[0]).toBeInTheDocument()
   })
+
+  it('adds target attribute to link if openInNewTab is true', () => {
+    const {getByRole} = render(<Todo {...defaultProps} />)
+    const link = getByRole('link', {name: 'Grade Plant a plant'})
+    expect(link.getAttribute('target')).toBe('_blank')
+  })
+
+  it('does not add target attribute to link if openInNewTab is false', () => {
+    const {getByRole} = render(<Todo {...defaultProps} openInNewTab={false} />)
+    const link = getByRole('link', {name: 'Grade Plant a plant'})
+    expect(link.getAttribute('target')).toBeNull()
+  })
 })

ui/features/k5_dashboard/react/__tests__/TodosPage.test.js

@@ -27,6 +27,13 @@ import TodosPage, {sortTodos} from '../TodosPage'
 
 const FETCH_TODOS_URL = /\/api\/v1\/users\/self\/todo.*/
 
+const getProps = overrides => ({
+  timeZone: 'America/Denver',
+  visible: true,
+  openTodosInNewTab: true,
+  ...overrides
+})
+
 describe('TodosPage', () => {
   beforeEach(() => {
     fetchMock.get(FETCH_TODOS_URL, MOCK_TODOS)
@@ -40,12 +47,12 @@ describe('TodosPage', () => {
 
   it('renders todo items for each todo once loaded', async () => {
     const {getAllByTestId, getAllByText, getByRole, findByRole, rerender, queryByRole} = render(
-      <TodosPage visible={false} timeZone="America/Denver" />
+      <TodosPage {...getProps({visible: false})} />
     )
     // Displays nothing when not visible
     expect(queryByRole('link')).not.toBeInTheDocument()
 
-    rerender(<TodosPage visible timeZone="America/Denver" />)
+    rerender(<TodosPage {...getProps()} />)
     // Displays loading skeletons when visible and todos are loading
     expect(getAllByTestId('todo-loading-skeleton').length).toBe(5)
     expect(getAllByText('Loading Todo Title')[0]).toBeInTheDocument()
@@ -63,12 +70,12 @@ describe('TodosPage', () => {
 
   it('renders an error if loading todos fails', async () => {
     fetchMock.get(FETCH_TODOS_URL, 500, {overwriteRoutes: true})
-    const {findAllByText} = render(<TodosPage visible timeZone="America/Denver" />)
+    const {findAllByText} = render(<TodosPage {...getProps()} />)
     expect((await findAllByText('Failed to load todos'))[0]).toBeInTheDocument()
   })
 
   it('ignores submitting-type todos', async () => {
-    const {findByRole, queryByText} = render(<TodosPage visible timeZone="America/Denver" />)
+    const {findByRole, queryByText} = render(<TodosPage {...getProps()} />)
     expect(await findByRole('link', {name: 'Grade Plant a plant'})).toBeInTheDocument()
     expect(queryByText('Long essay', {exact: false})).not.toBeInTheDocument()
   })
@@ -80,9 +87,7 @@ describe('Empty todos', () => {
   })
 
   it('shows an empty state if there are no todos', async () => {
-    const {getAllByText, findByText, findByTestId} = render(
-      <TodosPage visible timeZone="America/Denver" />
-    )
+    const {getAllByText, findByText, findByTestId} = render(<TodosPage {...getProps()} />)
     expect(getAllByText('Loading Todo Title')[0]).toBeInTheDocument()
 
     // Displays the empty state if no todos were found