From 555bcb8e4d1e6332661798741f3b1b9988c8d5ef Mon Sep 17 00:00:00 2001 From: Dominik Seeger <dominik.seeger@stud.uni-goettingen.de> Date: Tue, 2 Jul 2019 17:22:38 +0200 Subject: [PATCH] added shortcut for comment submitting, fixed comment drafts being displayed incorrectly --- .../feedback_labels/LabelSelector.vue | 15 ++++- .../submission_notes/base/CommentForm.vue | 1 + .../test_feedback_label_system.py | 55 +++++++++++++++---- functional_tests/util.py | 22 ++++---- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/frontend/src/components/feedback_labels/LabelSelector.vue b/frontend/src/components/feedback_labels/LabelSelector.vue index 37bb7e6d..7cd1c29c 100644 --- a/frontend/src/components/feedback_labels/LabelSelector.vue +++ b/frontend/src/components/feedback_labels/LabelSelector.vue @@ -11,6 +11,7 @@ item-value="pk" append-icon="search" placeholder="search for keywords" + @keyup.enter.ctrl.exact="submitFeedback" @input="addLabel" /> </v-flex> @@ -180,7 +181,7 @@ export default class LabelSelector extends Vue { removeLabel(pk: number) { if (this.assignedToFeedback) { if (!SubmissionNotes.state.changedLabels) { - SubmissionNotes.SET_FEEDBACK_LABELS(SubmissionNotes.state.origFeedback.labels) + SubmissionNotes.SET_FEEDBACK_LABELS([...SubmissionNotes.state.origFeedback.labels]) } SubmissionNotes.REMOVE_FEEDBACK_LABEL(pk) @@ -199,12 +200,24 @@ export default class LabelSelector extends Vue { if (!this.unchangedFeedbackLabels().includes(pk) && !this.addedFeedbackLabels().includes(pk)) { + if (!SubmissionNotes.state.changedLabels) { + SubmissionNotes.SET_FEEDBACK_LABELS([...SubmissionNotes.state.origFeedback.labels]) + } + SubmissionNotes.ADD_FEEDBACK_LABEL(pk) } } else { this.$emit("label-added", pk) } } + + /** + * Emits an event which tells the parent that + * the submit shortcut was pressed + */ + submitFeedback() { + this.$emit("submit-shortcut") + } } </script> diff --git a/frontend/src/components/submission_notes/base/CommentForm.vue b/frontend/src/components/submission_notes/base/CommentForm.vue index c4a8f5ed..83481e55 100644 --- a/frontend/src/components/submission_notes/base/CommentForm.vue +++ b/frontend/src/components/submission_notes/base/CommentForm.vue @@ -25,6 +25,7 @@ :labelsRemoved="labelsRemoved" @label-added="labelAdded" @label-removed="labelRemoved" + @submit-shortcut="submitFeedback" /> </v-flex> <v-flex lg2 ma-0> diff --git a/functional_tests/test_feedback_label_system.py b/functional_tests/test_feedback_label_system.py index 44d63cce..06883e3c 100644 --- a/functional_tests/test_feedback_label_system.py +++ b/functional_tests/test_feedback_label_system.py @@ -193,12 +193,21 @@ class FeedbackLabelSystemTest(LiveServerTestCase): login(self.browser, self.live_server_url, username, password) go_to_subscription(self, stage='validate') - code = reconstruct_submission_code(self) self.remove_label_from_feedback('test') + added = self.browser.find_elements_by_xpath( + '//div[@id="feedback-label-selector"]//div[contains(text(), "WILL BE ADDED")]/..//*' + ) removed = self.browser.find_elements_by_xpath( '//div[@id="feedback-label-selector"]//div[contains(text(), "WILL BE REMOVED")]/..//*' ) + unchanged = self.browser.find_elements_by_xpath( + '//div[@id="feedback-label-selector"]//div[contains(text(), "UNCHANGED")]/..//*' + ) + + print(unchanged) self.assertGreater(len(removed), 1) + self.assertEqual(len(unchanged), 1) + self.assertEqual(len(added), 1) self.browser.find_element_by_id('submit-feedback').click() sub_url = 'subscription/' + str(self.sub_type.pk) + '/ended' @@ -229,19 +238,29 @@ class FeedbackLabelSystemTest(LiveServerTestCase): login(self.browser, self.live_server_url, username, password) go_to_subscription(self, stage='validate') - code = reconstruct_submission_code(self) self.assign_label_to_feedback('add') added = self.browser.find_elements_by_xpath( '//div[@id="feedback-label-selector"]//div[contains(text(), "WILL BE ADDED")]/..//*' ) + removed = self.browser.find_elements_by_xpath( + '//div[@id="feedback-label-selector"]//div[contains(text(), "WILL BE REMOVED")]/..//*' + ) + unchanged = self.browser.find_elements_by_xpath( + '//div[@id="feedback-label-selector"]//div[contains(text(), "UNCHANGED")]/..//*' + ) + + self.assertEqual(len(removed), 1) self.assertGreater(len(added), 1) + self.assertGreater(len(unchanged), 1) self.browser.find_element_by_id('submit-feedback').click() sub_url = 'subscription/' + str(self.sub_type.pk) + '/ended' WebDriverWait(self.browser, 10).until(ec.url_contains(sub_url)) - label = FeedbackLabel.objects.get(name='add') + new_label = FeedbackLabel.objects.get(name='add') + old_label = FeedbackLabel.objects.get(name='test') - self.assertEqual(len(label.feedback.all()), 1) + self.assertEqual(len(old_label.feedback.all()), 1) + self.assertEqual(len(new_label.feedback.all()), 1) def test_can_assign_label_to_comment(self): self._login() @@ -286,13 +305,21 @@ class FeedbackLabelSystemTest(LiveServerTestCase): login(self.browser, self.live_server_url, username, password) go_to_subscription(self, stage='validate') - code = reconstruct_submission_code(self) self.browser.find_element_by_id('feedback-visibility-toggle').click() self.remove_label_from_comment_line(comment_line, 'test') + added = self.browser.find_elements_by_xpath( + f'//tr[@id="sub-line-{comment_line}"]//div[contains(text(), "WILL BE ADDED")]/..//*' + ) removed = self.browser.find_elements_by_xpath( - '//div[@id="feedback-label-selector"]//div[contains(text(), "WILL BE REMOVED")]/..//*' + f'//tr[@id="sub-line-{comment_line}"]//div[contains(text(), "WILL BE REMOVED")]/..//*' ) - self.assertGreaterEqual(len(removed), 1) + unchanged = self.browser.find_elements_by_xpath( + f'//tr[@id="sub-line-{comment_line}"]//div[contains(text(), "UNCHANGED")]/..//*' + ) + + self.assertGreater(len(removed), 1) + self.assertEqual(len(added), 1) + self.assertEqual(len(unchanged), 1) self.browser.find_element_by_id('submit-feedback').click() sub_url = 'subscription/' + str(self.sub_type.pk) + '/ended' @@ -325,13 +352,21 @@ class FeedbackLabelSystemTest(LiveServerTestCase): login(self.browser, self.live_server_url, username, password) go_to_subscription(self, stage='validate') - code = reconstruct_submission_code(self) self.browser.find_element_by_id('feedback-visibility-toggle').click() self.assign_label_to_comment_line(comment_line, 'add') added = self.browser.find_elements_by_xpath( - '//div[@id="feedback-label-selector"]//div[contains(text(), "WILL BE ADDED")]/..//*' + f'//tr[@id="sub-line-{comment_line}"]//div[contains(text(), "WILL BE ADDED")]/..//*' + ) + removed = self.browser.find_elements_by_xpath( + f'//tr[@id="sub-line-{comment_line}"]//div[contains(text(), "WILL BE REMOVED")]/..//*' ) - self.assertGreaterEqual(len(added), 1) + unchanged = self.browser.find_elements_by_xpath( + f'//tr[@id="sub-line-{comment_line}"]//div[contains(text(), "UNCHANGED")]/..//*' + ) + + self.assertEqual(len(removed), 1) + self.assertGreater(len(unchanged), 1) + self.assertGreater(len(added), 1) self.browser.find_element_by_id('submit-feedback').click() sub_url = 'subscription/' + str(self.sub_type.pk) + '/ended' diff --git a/functional_tests/util.py b/functional_tests/util.py index be898df2..9fd0e6ed 100644 --- a/functional_tests/util.py +++ b/functional_tests/util.py @@ -86,26 +86,28 @@ def query_returns_object(model_class, **kwargs): # stage can be 'initial', 'validate', or 'conflict' -def go_to_subscription(self, stage='initial', sub_type=None): - WebDriverWait(self.browser, 10).until(subscriptions_loaded_cond(self.browser)) - tasks = self.browser.find_element_by_name('subscription-list') +def go_to_subscription(test_class_instance, stage='initial', sub_type=None): + WebDriverWait(test_class_instance.browser, 10).until( + subscriptions_loaded_cond(test_class_instance.browser) + ) + tasks = test_class_instance.browser.find_element_by_name('subscription-list') tab = tasks.find_element_by_xpath(f'//*[contains(text(), "{stage}")]') tab.click() - sub_type = sub_type if sub_type is not None else self.sub_type + sub_type = sub_type if sub_type is not None else test_class_instance.sub_type sub_type_xpath = f'//*[contains(text(), "{sub_type.name}") ' \ f'and not(contains(@class, "inactive"))]' - WebDriverWait(self.browser, 10).until( + WebDriverWait(test_class_instance.browser, 10).until( ec.element_to_be_clickable((By.XPATH, sub_type_xpath)), message="SubmissionType not clickable" ) sub_type_el = tasks.find_element_by_xpath(sub_type_xpath) sub_type_el.click() - WebDriverWait(self.browser, 10).until(ec.url_contains('subscription')) + WebDriverWait(test_class_instance.browser, 10).until(ec.url_contains('subscription')) -def reconstruct_submission_code(self): - sub_table = self.browser.find_element_by_class_name('submission-table') +def reconstruct_submission_code(test_class_instance): + sub_table = test_class_instance.browser.find_element_by_class_name('submission-table') lines = sub_table.find_elements_by_tag_name('tr') line_no_code_pairs = [ (line.get_attribute('id'), @@ -119,11 +121,11 @@ def reconstruct_submission_code(self): return '\n'.join(code_lines) -def wait_until_code_changes(self, code): +def wait_until_code_changes(test_class_instance, code): def condition(*args): try: # code might change during the call resulting in the exception - new_code = reconstruct_submission_code(self) + new_code = reconstruct_submission_code(test_class_instance) except StaleElementReferenceException: return False return code != new_code -- GitLab