-
Notifications
You must be signed in to change notification settings - Fork 42
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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) | ||
|
@@ -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 | ||
|
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( |
||
|
||
@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): | ||
|
@@ -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) | ||
|
@@ -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 | ||
|
@@ -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): | ||
|
There was a problem hiding this comment.
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 theSourceWidget
?