Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update gui instead of sync when file missing #724

Merged
merged 3 commits into from
Jan 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 33 additions & 11 deletions securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -1836,6 +1836,7 @@ def __init__(
file_uuid: str,
controller: Controller,
file_ready_signal: pyqtBoundSignal,
file_missing: pyqtBoundSignal,
index: int,
) -> None:
"""
Expand All @@ -1852,8 +1853,9 @@ def __init__(
self.setStyleSheet(self.CSS)
file_description_font = QFont()
file_description_font.setLetterSpacing(QFont.AbsoluteSpacing, self.FILE_FONT_SPACING)
file_buttons_font = QFont()
file_buttons_font.setLetterSpacing(QFont.AbsoluteSpacing, self.FILE_OPTIONS_FONT_SPACING)
self.file_buttons_font = QFont()
self.file_buttons_font.setLetterSpacing(
QFont.AbsoluteSpacing, self.FILE_OPTIONS_FONT_SPACING)

# Set layout
layout = QHBoxLayout()
Expand All @@ -1876,15 +1878,15 @@ def __init__(
self.download_button.setObjectName('download_button')
self.download_button.setSizePolicy(QSizePolicy.Fixed, QSizePolicy.Fixed)
self.download_button.setIcon(load_icon('download_file.svg'))
self.download_button.setFont(file_buttons_font)
self.download_button.setFont(self.file_buttons_font)
self.download_animation = load_movie("download_file.gif")
self.export_button = QPushButton(_('EXPORT'))
self.export_button.setObjectName('export_print')
self.export_button.setFont(file_buttons_font)
self.export_button.setFont(self.file_buttons_font)
self.middot = QLabel("·")
self.print_button = QPushButton(_('PRINT'))
self.print_button.setObjectName('export_print')
self.print_button.setFont(file_buttons_font)
self.print_button.setFont(self.file_buttons_font)
file_options_layout.addWidget(self.download_button)
file_options_layout.addWidget(self.export_button)
file_options_layout.addWidget(self.middot)
Expand Down Expand Up @@ -1937,6 +1939,7 @@ def __init__(

# Connect signals to slots
file_ready_signal.connect(self._on_file_downloaded, type=Qt.QueuedConnection)
file_missing.connect(self._on_file_missing, type=Qt.QueuedConnection)

def eventFilter(self, obj, event):
if event.type() == QEvent.MouseButtonPress:
Expand All @@ -1957,17 +1960,33 @@ def _on_file_downloaded(self, file_uuid: str) -> None:
self.print_button.show()
self.file_name.show()

@pyqtSlot(str)
def _on_file_missing(self, file_uuid: str) -> None:
if file_uuid == self.file.uuid:
self.file = self.controller.get_file(self.file.uuid)
if not self.file.is_downloaded:
self.download_animation.stop()
self.download_button.setText(_('DOWNLOAD'))
self.download_button.setIcon(load_icon('download_file.svg'))
self.download_button.setStyleSheet('color: #2a319d')
self.download_button.setFont(self.file_buttons_font)
self.download_button.show()
self.no_file_name.hide()
self.export_button.hide()
self.middot.hide()
self.print_button.hide()
self.file_name.hide()
self.no_file_name.show()

@pyqtSlot()
def _on_export_clicked(self):
"""
Called when the export button is clicked.
"""
if not self.controller.downloaded_file_exists(self.file.uuid):
self.controller.sync_api()
return

dialog = ExportDialog(self.controller, self.file.uuid,
self.file.original_filename)
dialog = ExportDialog(self.controller, self.file.uuid, self.file.original_filename)
dialog.show()
dialog.export()
dialog.exec()
Expand All @@ -1978,7 +1997,6 @@ def _on_print_clicked(self):
Called when the print button is clicked.
"""
if not self.controller.downloaded_file_exists(self.file.uuid):
self.controller.sync_api()
return

dialog = PrintDialog(self.controller, self.file.uuid)
Expand Down Expand Up @@ -2495,8 +2513,12 @@ def add_file(self, file: File, index):
"""
Add a file from the source.
"""
conversation_item = FileWidget(file.uuid, self.controller, self.controller.file_ready,
index)
conversation_item = FileWidget(
file.uuid,
self.controller,
self.controller.file_ready,
self.controller.file_missing,
index)
self.conversation_layout.insertWidget(index, conversation_item, alignment=Qt.AlignLeft)
self.current_messages[file.uuid] = conversation_item

Expand Down
16 changes: 9 additions & 7 deletions securedrop_client/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ class Controller(QObject):
"""
file_ready = pyqtSignal(str)

"""
This signal indicates that a file needs to be redownloaded by emitting the file's UUID.
"""
file_missing = pyqtSignal(str)

"""
This signal indicates that a message has been successfully downloaded by emitting the message's
UUID as a string.
Expand Down Expand Up @@ -570,17 +575,17 @@ def on_reply_download_failure(self, exception: Exception) -> None:

def downloaded_file_exists(self, file_uuid: str) -> bool:
'''
Check if the file specified by file_uuid exists. If it doesn't sync the api so that any
missing files, including this one, are updated to be re-downloaded.
Check if the file specified by file_uuid exists. If it doesn't update the local db and
GUI to show the file as not downloaded.
'''
file = self.get_file(file_uuid)
fn_no_ext, dummy = os.path.splitext(os.path.splitext(file.filename)[0])
filepath = os.path.join(self.data_dir, fn_no_ext)
if not os.path.exists(filepath):
self.gui.update_error_status(_(
'File does not exist in the data directory. Please try re-downloading.'))
logger.debug('Cannot find {} in the data directory. File does not exist.'.format(
file.original_filename))
storage.update_missing_files(self.data_dir, self.session)
self.file_missing.emit(file.uuid)
return False
return True

Expand Down Expand Up @@ -623,7 +628,6 @@ def on_file_open(self, file_uuid: str) -> None:
logger.info('Opening file "{}".'.format(file.original_filename))

if not self.downloaded_file_exists(file.uuid):
self.sync_api()
return

if not self.qubes:
Expand Down Expand Up @@ -658,7 +662,6 @@ def export_file_to_usb_drive(self, file_uuid: str, passphrase: str) -> None:
logger.info('Exporting file {}'.format(file.original_filename))

if not self.downloaded_file_exists(file.uuid):
self.sync_api()
return

if not self.qubes:
Expand All @@ -678,7 +681,6 @@ def print_file(self, file_uuid: str) -> None:
logger.info('Printing file {}'.format(file.original_filename))

if not self.downloaded_file_exists(file.uuid):
self.sync_api()
return

if not self.qubes:
Expand Down
2 changes: 1 addition & 1 deletion securedrop_client/resources/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def load_css(name: str) -> str:
return resource_string(__name__, "css/" + name).decode('utf-8')


def load_movie(name: str) -> str:
def load_movie(name: str) -> QMovie:
"""
Return a GIF animation to use in the UI.
"""
Expand Down
84 changes: 60 additions & 24 deletions tests/gui/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -1380,7 +1380,7 @@ def test_FileWidget_init_file_not_downloaded(mocker, source, session):
get_file = mocker.MagicMock(return_value=file)
controller = mocker.MagicMock(get_file=get_file)

fw = FileWidget('mock', controller, mocker.MagicMock(), 0)
fw = FileWidget('mock', controller, mocker.MagicMock(), mocker.MagicMock(), 0)

assert fw.controller == controller
assert fw.file.is_downloaded is False
Expand All @@ -1403,7 +1403,7 @@ def test_FileWidget_init_file_downloaded(mocker, source, session):
get_file = mocker.MagicMock(return_value=file)
controller = mocker.MagicMock(get_file=get_file)

fw = FileWidget('mock', controller, mocker.MagicMock(), 0)
fw = FileWidget('mock', controller, mocker.MagicMock(), mocker.MagicMock(), 0)

assert fw.controller == controller
assert fw.file.is_downloaded is True
Expand All @@ -1419,8 +1419,6 @@ def test_FileWidget_event_handler(mocker, session, source):
"""
Left click on filename should trigger an open.
"""
mock_signal = mocker.MagicMock() # not important for this test

file_ = factory.File(source=source['source'],
is_downloaded=False,
is_decrypted=None)
Expand All @@ -1432,7 +1430,7 @@ def test_FileWidget_event_handler(mocker, session, source):
test_event = QEvent(QEvent.MouseButtonPress)
test_event.button = mocker.MagicMock(return_value=Qt.LeftButton)

fw = FileWidget(file_.uuid, mock_controller, mock_signal, 0)
fw = FileWidget(file_.uuid, mock_controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw._on_left_click = mocker.MagicMock()

fw.eventFilter(fw, test_event)
Expand All @@ -1444,8 +1442,6 @@ def test_FileWidget_on_left_click_download(mocker, session, source):
Left click on download when file is not downloaded should trigger
a download.
"""
mock_signal = mocker.MagicMock() # not important for this test

file_ = factory.File(source=source['source'],
is_downloaded=False,
is_decrypted=None)
Expand All @@ -1455,7 +1451,7 @@ def test_FileWidget_on_left_click_download(mocker, session, source):
mock_get_file = mocker.MagicMock(return_value=file_)
mock_controller = mocker.MagicMock(get_file=mock_get_file)

fw = FileWidget(file_.uuid, mock_controller, mock_signal, 0)
fw = FileWidget(file_.uuid, mock_controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw.download_button = mocker.MagicMock()
mock_get_file.assert_called_once_with(file_.uuid)
mock_get_file.reset_mock()
Expand All @@ -1474,16 +1470,14 @@ def test_FileWidget_start_button_animation(mocker, session, source):
"""
Ensure widget state is updated when this method is called.
"""
mock_signal = mocker.MagicMock() # not important for this test

file_ = factory.File(source=source['source'],
is_downloaded=False,
is_decrypted=None)
session.add(file_)
session.commit()
mock_get_file = mocker.MagicMock(return_value=file_)
mock_controller = mocker.MagicMock(get_file=mock_get_file)
fw = FileWidget(file_.uuid, mock_controller, mock_signal, 0)
fw = FileWidget(file_.uuid, mock_controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw.download_button = mocker.MagicMock()
fw.start_button_animation()
# Check indicators of activity have been updated.
Expand All @@ -1496,16 +1490,14 @@ def test_FileWidget_on_left_click_open(mocker, session, source):
"""
Left click on open when file is downloaded should trigger an open.
"""
mock_signal = mocker.MagicMock() # not important for this test

file_ = factory.File(source=source['source'], is_downloaded=True)
session.add(file_)
session.commit()

mock_get_file = mocker.MagicMock(return_value=file_)
mock_controller = mocker.MagicMock(get_file=mock_get_file)

fw = FileWidget(file_.uuid, mock_controller, mock_signal, 0)
fw = FileWidget(file_.uuid, mock_controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw._on_left_click()
fw.controller.on_file_open.assert_called_once_with(file_.uuid)

Expand All @@ -1515,8 +1507,6 @@ def test_FileWidget_set_button_animation_frame(mocker, session, source):
Left click on download when file is not downloaded should trigger
a download.
"""
mock_signal = mocker.MagicMock() # not important for this test

file_ = factory.File(source=source['source'],
is_downloaded=False,
is_decrypted=None)
Expand All @@ -1526,7 +1516,7 @@ def test_FileWidget_set_button_animation_frame(mocker, session, source):
mock_get_file = mocker.MagicMock(return_value=file_)
mock_controller = mocker.MagicMock(get_file=mock_get_file)

fw = FileWidget(file_.uuid, mock_controller, mock_signal, 0)
fw = FileWidget(file_.uuid, mock_controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw.download_button = mocker.MagicMock()
fw.set_button_animation_frame(1)
assert fw.download_button.setIcon.call_count == 1
Expand All @@ -1541,7 +1531,7 @@ def test_FileWidget_update(mocker, session, source):
session.commit()
get_file = mocker.MagicMock(return_value=file)
controller = mocker.MagicMock(get_file=get_file)
fw = FileWidget(file.uuid, controller, mocker.MagicMock(), 0)
fw = FileWidget(file.uuid, controller, mocker.MagicMock(), mocker.MagicMock(), 0)

fw.update()

Expand All @@ -1561,7 +1551,7 @@ def test_FileWidget_on_file_download_updates_items_when_uuid_matches(mocker, sou
get_file = mocker.MagicMock(return_value=file)
controller = mocker.MagicMock(get_file=get_file)

fw = FileWidget(file.uuid, controller, mocker.MagicMock(), 0)
fw = FileWidget(file.uuid, controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw.update = mocker.MagicMock()

fw._on_file_downloaded(file.uuid)
Expand All @@ -1587,7 +1577,7 @@ def test_FileWidget_on_file_download_updates_items_when_uuid_does_not_match(
get_file = mocker.MagicMock(return_value=file)
controller = mocker.MagicMock(get_file=get_file)

fw = FileWidget(file.uuid, controller, mocker.MagicMock(), 0)
fw = FileWidget(file.uuid, controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw.clear = mocker.MagicMock()
fw.update = mocker.MagicMock()

Expand All @@ -1602,6 +1592,52 @@ def test_FileWidget_on_file_download_updates_items_when_uuid_does_not_match(
assert not fw.file_name.isHidden()


def test_FileWidget_on_file_missing_show_download_button_when_uuid_matches(mocker, source, session):
"""
The _on_file_missing method should update the FileWidget when uuid matches.
"""
file = factory.File(source=source['source'], is_decrypted=None, is_downloaded=False)
session.add(file)
session.commit()

get_file = mocker.MagicMock(return_value=file)
controller = mocker.MagicMock(get_file=get_file)

fw = FileWidget(file.uuid, controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw.update = mocker.MagicMock()

fw._on_file_missing(file.uuid)

assert not fw.download_button.isHidden()
assert fw.export_button.isHidden()
assert fw.middot.isHidden()
assert fw.print_button.isHidden()
assert not fw.no_file_name.isHidden()
assert fw.file_name.isHidden()
assert fw.download_animation.state() == QMovie.NotRunning


def test_FileWidget_on_file_missing_does_not_show_download_button_when_uuid_does_not_match(
mocker, homedir, session, source,
):
"""
The _on_file_missing method should not update the FileWidget when uuid doesn't match.
"""
file = factory.File(source=source['source'])
session.add(file)
session.commit()

get_file = mocker.MagicMock(return_value=file)
controller = mocker.MagicMock(get_file=get_file)

fw = FileWidget(file.uuid, controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw.download_button.show = mocker.MagicMock()

fw._on_file_missing('not a matching uuid')

fw.download_button.show.assert_not_called()


def test_FileWidget__on_export_clicked(mocker, session, source):
"""
Ensure preflight checks start when the EXPORT button is clicked and that password is requested
Expand All @@ -1613,7 +1649,7 @@ def test_FileWidget__on_export_clicked(mocker, session, source):
get_file = mocker.MagicMock(return_value=file)
controller = mocker.MagicMock(get_file=get_file)

fw = FileWidget(file.uuid, controller, mocker.MagicMock(), 0)
fw = FileWidget(file.uuid, controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw.update = mocker.MagicMock()
mocker.patch('securedrop_client.gui.widgets.QDialog.exec')
controller.run_export_preflight_checks = mocker.MagicMock()
Expand All @@ -1640,7 +1676,7 @@ def test_FileWidget__on_export_clicked_missing_file(mocker, session, source):
get_file = mocker.MagicMock(return_value=file)
controller = mocker.MagicMock(get_file=get_file)

fw = FileWidget(file.uuid, controller, mocker.MagicMock(), 0)
fw = FileWidget(file.uuid, controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw.update = mocker.MagicMock()
mocker.patch('securedrop_client.gui.widgets.QDialog.exec')
controller.run_export_preflight_checks = mocker.MagicMock()
Expand All @@ -1664,7 +1700,7 @@ def test_FileWidget__on_print_clicked(mocker, session, source):
get_file = mocker.MagicMock(return_value=file)
controller = mocker.MagicMock(get_file=get_file)

fw = FileWidget(file.uuid, controller, mocker.MagicMock(), 0)
fw = FileWidget(file.uuid, controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw.update = mocker.MagicMock()
mocker.patch('securedrop_client.gui.widgets.QDialog.exec')
controller.print_file = mocker.MagicMock()
Expand All @@ -1691,7 +1727,7 @@ def test_FileWidget__on_print_clicked_missing_file(mocker, session, source):
get_file = mocker.MagicMock(return_value=file)
controller = mocker.MagicMock(get_file=get_file)

fw = FileWidget(file.uuid, controller, mocker.MagicMock(), 0)
fw = FileWidget(file.uuid, controller, mocker.MagicMock(), mocker.MagicMock(), 0)
fw.update = mocker.MagicMock()
mocker.patch('securedrop_client.gui.widgets.QDialog.exec')
controller.print_file = mocker.MagicMock()
Expand Down
Loading