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

Remove source db object from SourceWidget #967

Closed
wants to merge 4 commits into from
Closed
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
170 changes: 96 additions & 74 deletions securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@

from securedrop_client import __version__ as sd_version
from securedrop_client.db import DraftReply, Source, Message, File, Reply, User
from securedrop_client.storage import source_exists
from securedrop_client.export import ExportStatus, ExportError
from securedrop_client.gui import SecureQLabel, SvgLabel, SvgPushButton, SvgToggleButton
from securedrop_client.logic import Controller
Expand Down Expand Up @@ -912,46 +911,44 @@ def setup(self, controller):

def update(self, sources: List[Source]) -> List[str]:
"""
Update the list with the passed in list of sources.
Update the list with the passed in list of sources and return the list of uuids of sources
that were deleted so that later their corresponding conversation widgets can be deleted.
"""
# Delete widgets that no longer exist in source list
source_uuids = [source.uuid for source in sources]
deleted_uuids = []

# Update all existing widgets in case there was an update made via the Journalist
# Interface or by another instance of the client.
#
# Otherwise, delete all existing widgets that are not in the provided `sources` list.
source_uuids = [source.uuid for source in sources]
for i in range(self.count()):
list_item = self.item(i)
list_widget = self.itemWidget(list_item)

if list_widget and list_widget.source_uuid not in source_uuids:
if not list_widget:
continue

if list_widget.source_uuid in source_uuids:
list_widget.update()
else:
if list_item.isSelected():
self.setCurrentItem(None)

try:
del self.source_widgets[list_widget.source_uuid]
deleted_uuids.append(list_widget.source_uuid)
except KeyError:
pass
finally:
self.takeItem(i)
list_widget.deleteLater()

# Create new widgets for new sources
self.takeItem(i)
deleted_uuids.append(list_widget.source_uuid)
list_widget.deleteLater()

# Create and add new widgets to the source list for new sources.
widget_uuids = [self.itemWidget(self.item(i)).source_uuid for i in range(self.count())]
for source in sources:
if source.uuid in widget_uuids:
try:
self.source_widgets[source.uuid].update()
except sqlalchemy.exc.InvalidRequestError as e:
logger.error(
"Could not update SourceWidget for source %s; deleting it. Error was: %s",
source.uuid,
e
)
deleted_uuids.append(source.uuid)
self.source_widgets[source.uuid].deleteLater()
del self.source_widgets[list_widget.source_uuid]
else:
new_source = SourceWidget(self.controller, source)
self.source_widgets[source.uuid] = new_source
for source_uuid in source_uuids:
if source_uuid not in widget_uuids:
new_source = SourceWidget(self.controller, source_uuid)
self.source_widgets[source_uuid] = new_source

list_item = QListWidgetItem()
self.insertItem(0, list_item)
Expand All @@ -961,20 +958,41 @@ def update(self, sources: List[Source]) -> List[str]:
return deleted_uuids

def get_current_source(self):
source_item = self.currentItem()
source_widget = self.itemWidget(source_item)
if source_widget and source_exists(self.controller.session, source_widget.source_uuid):
return source_widget.source
try:
source_item = self.currentItem()
source_widget = self.itemWidget(source_item)
if source_widget:
return self.controller.get_source(source_widget.source_uuid)
except sqlalchemy.exc.InvalidRequestError:
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass so that we let the next sync delete the SourceWidget?


def set_snippet(self, source_uuid, message_uuid, content):
"""
Given a UUID of a source, if the referenced message is the latest
message, then update the source's preview snippet to the referenced
content.
"""
source_widget = self.source_widgets.get(source_uuid)
def get_source_widget(self, source_uuid: str) -> QListWidget:
'''
First try to get the source widget from the cache, then look for it in the SourceList.
'''
try:
source_widget = self.source_widgets[source_uuid]
return source_widget
except KeyError:
pass

for i in range(self.count()):
list_item = self.item(i)
source_widget = self.itemWidget(list_item)
if source_widget and source_widget.source_uuid == source_uuid:
return source_widget
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clarifying: under which circumstances can the source widget be in the cache but not the source list?

Copy link
Contributor Author

@sssoleileraaa sssoleileraaa Mar 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could rely completely on the cache. I abstracted getting the source widget here because I was thinking we didn't need both the qt list and the cache since we have to loop through all items in the qt list (SourceList) anyway. But after thinking about this more, we have future plans to modify server code so that it only returns sources that have updates, and that's when the cache will come in handy (because we won't have to loop through the entire source list anymore and update all the source widgets). Long story short: I can update this method so that there's no fall back if we try to get a source widget that hasn't been added to the cache yet (since we add items to the cache before adding them to the qt list, this currently can't happen, but, if we ever switch the order of this by accident, having a fallback would ensure that we'll be able to get the selected source widget in the list)


@pyqtSlot(str, str, str)
def set_snippet(self, source_uuid: str, collection_item_uuid: str, content: str) -> None:
'''
Set the source widget's preview snippet with the supplied content.

Note: The signal's `collection_item_uuid` is not needed for setting the preview snippet. It
is used by other signal handlers.
'''
source_widget = self.get_source_widget(source_uuid)
if source_widget:
source_widget.set_snippet(source_uuid, message_uuid, content)
source_widget.set_snippet(source_uuid, content)


class SourceWidget(QWidget):
Expand Down Expand Up @@ -1046,16 +1064,14 @@ class SourceWidget(QWidget):
PREVIEW_WIDTH = 412
PREVIEW_HEIGHT = 60

def __init__(self, controller: Controller, source: Source):
def __init__(self, controller: Controller, source_uuid: str):
super().__init__()

self.controller = controller
self.controller.source_deleted.connect(self._on_source_deleted)

# Store source
self.source_uuid = source.uuid
self.source = source
self.source_uuid = source.uuid
# Store source uuid
self.source_uuid = source_uuid

# Set styles
self.setStyleSheet(self.CSS)
Expand All @@ -1081,8 +1097,15 @@ def __init__(self, controller: Controller, source: Source):
gutter_layout = QVBoxLayout(self.gutter)
gutter_layout.setContentsMargins(0, 0, 0, 0)
gutter_layout.setSpacing(0)
self.star = StarToggleButton(self.controller, self.source)
gutter_layout.addWidget(self.star)

# TODO: No longer pass the source db object to the star toggle button
try:
source = self.controller.get_source(self.source_uuid)
self.star = StarToggleButton(self.controller, source)
gutter_layout.addWidget(self.star)
except sqlalchemy.exc.InvalidRequestError as e:
logger.error(f'Cannot update star icon for source: {self.source_uuid}: {e}')

gutter_layout.addStretch()

# Set up summary
Expand Down Expand Up @@ -1139,51 +1162,50 @@ def __init__(self, controller: Controller, source: Source):

def update(self):
"""
Updates the displayed values with the current values from self.source.
Syncs the source widget with the source stored in the database. If the source in the
database no longer exists, then log an error and wait for the next sync to delete this
widget.
"""
try:
self.controller.session.refresh(self.source)
self.timestamp.setText(_(arrow.get(self.source.last_updated).format('DD MMM')))
self.name.setText(self.source.journalist_designation)
self.set_snippet(self.source.uuid)
if self.source.document_count == 0:
source = self.controller.get_source(self.source_uuid)

self.timestamp.setText(_(arrow.get(source.last_updated).format('DD MMM')))
self.name.setText(source.journalist_designation)

if not source.collection:
self.set_snippet(source.uuid, '')
else:
last_collection_obj = source.collection[-1]
self.set_snippet(source.uuid, str(last_collection_obj))

if source.document_count == 0:
self.paperclip.hide()

self.star.update()
except sqlalchemy.exc.InvalidRequestError as e:
logger.error(f"Could not update SourceWidget for source {self.source_uuid}: {e}")
raise
logger.error(f'Could not update SourceWidget for source {self.source_uuid}: {e}')

def set_snippet(self, source: str, uuid: str = None, content: str = None):
def set_snippet(self, source_uuid: str, content: str):
"""
Update the preview snippet only if the new message is for the
referenced source and there's a source collection. If a uuid and
content are passed then use these, otherwise default to whatever the
latest item in the conversation might be.
Update the preview snippet if the source_uuid matches our own.
"""
if source != self.source_uuid:
if source_uuid != self.source_uuid:
return

self.preview.setText("")

try:
self.controller.session.refresh(self.source)
if not self.source.collection:
return
last_collection_object = self.source.collection[-1]
if uuid == last_collection_object.uuid and content:
self.preview.setText(content)
else:
self.preview.setText(str(last_collection_object))
except sqlalchemy.exc.InvalidRequestError as e:
logger.error(f"Could not update snippet for source {self.source_uuid}: {e}")
self.preview.setText(content)

def delete_source(self, event):
if self.controller.api is None:
self.controller.on_action_requiring_login()
return
else:
messagebox = DeleteSourceMessageBox(self.source, self.controller)
messagebox.launch()
# TODO: No longer pass the source db object to DeleteSourceMessageBox
try:
source = self.controller.get_source(self.source_uuid)
messagebox = DeleteSourceMessageBox(source, self.controller)
messagebox.launch()
except sqlalchemy.exc.InvalidRequestError as e:
logger.error(f'Cannot create messagebox for source: {self.source_uuid}: {e}')

@pyqtSlot(str)
def _on_source_deleted(self, source_uuid: str):
Expand Down
5 changes: 5 additions & 0 deletions securedrop_client/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,11 @@ def on_reply_failure(
if isinstance(exception, SendReplyJobError):
self.reply_failed.emit(exception.reply_uuid)

def get_source(self, source_uuid: str) -> db.Source:
source = storage.get_source(self.session, source_uuid)
self.session.refresh(source)
return source

def get_file(self, file_uuid: str) -> db.File:
file = storage.get_file(self.session, file_uuid)
self.session.refresh(file)
Expand Down
4 changes: 4 additions & 0 deletions securedrop_client/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,10 @@ def source_exists(session: Session, source_uuid: str) -> bool:
return False


def get_source(session: Session, uuid: str) -> Source:
return session.query(Source).filter_by(uuid=uuid).one()


def get_file(session: Session, uuid: str) -> File:
return session.query(File).filter_by(uuid=uuid).one()

Expand Down
Loading