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

Game 18568 fast note creation ownership #72

Open
wants to merge 6 commits into
base: dynamite-lib
Choose a base branch
from
Open
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
72 changes: 52 additions & 20 deletions python/activity_stream/activity_stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ class ActivityStreamWidget(QtGui.QWidget):
:signal playback_requested(dict): Fires when someone clicks the playback url
on a version. Returns a shotgun dictionary with information
about the version.
:signal entity_created(object): Fires when a Note or Reply entity is created by
:signal new_entity_requested_internally(str, int): Fires when the user has
triggered the creation of a new Note or Reply entity by interacting
with this Widget. This can be used to capture state relative to the Note
at the time of the action, since the creation itself is asynchronous.
:signal entity_created(object, object, int): Fires when a Note or Reply entity is created by
an underlying widget within the activity stream. Returns a Shotgun dictionary
with information about the new Entity.
:ivar reply_dialog: When a ReplyDialog is active it can be accessed here. If there
Expand Down Expand Up @@ -70,11 +74,25 @@ class ActivityStreamWidget(QtGui.QWidget):
# Called when a note is loaded either from remote download or the cache
note_arrived = QtCore.Signal(int)

# Called when the user has initiated the creation of a new Note by interacting
# with this widget.
new_entity_requested_internally = QtCore.Signal(str, int)

# Called when entity creation triggered by this widget has completed. Passes
# the entity info dict and the creation request that was sent with the matching
# new_entity_requested_internally.
entity_created_internally = QtCore.Signal(object, int)

# Called when a note creation triggered via `create_note` has completed. Passes
# the entity info and the userdata that was passed to `create_note`.
note_created_externally = QtCore.Signal(object, object)

# Emitted when a Note or Reply entity is created. The
# entity type as a string and id as an int will be
# provided.
#
# dict(entity_type="Note", id=1234)
#
entity_created = QtCore.Signal(object)

def __init__(self, parent):
Expand Down Expand Up @@ -125,6 +143,7 @@ def __init__(self, parent):
self._data_manager.note_arrived.connect(self._process_new_note)
self._data_manager.update_arrived.connect(self._process_new_data)
self._data_manager.thumbnail_arrived.connect(self._process_thumbnail)
self.ui.note_widget.new_entity_requested.connect(self._on_new_entity_requested_internally)
self.ui.note_widget.entity_created.connect(self._on_entity_created)
self.ui.note_widget.data_updated.connect(self.rescan)

Expand Down Expand Up @@ -157,11 +176,12 @@ def __init__(self, parent):
# attachments that this widget didn't explicitly handle itself prior to
# submission.
self._pre_submit_callback = None
self.reply_dialog.note_widget.new_entity_requested.connect(self._on_new_entity_requested_internally)
self.reply_dialog.note_widget.entity_created.connect(self._on_entity_created)

shotgun_model = sgtk.platform.import_framework("tk-framework-shotgunutils", "shotgun_model")
self._note_creator = shotgun_model.NoteCreator()
self._note_creator.entity_created.connect(self._on_note_created)
self._note_creator.entity_created.connect(self._on_note_created_externally)

def set_bg_task_manager(self, task_manager):
"""
Expand Down Expand Up @@ -705,6 +725,7 @@ def show_new_note_dialog(self, modal=True):

note_dialog = note_input_widget.NoteInputDialog(parent=self)
note_dialog.entity_created.connect(self._on_entity_created)
note_dialog.new_entity_requested.connect(self._on_new_entity_requested_internally)
note_dialog.data_updated.connect(self.rescan)
note_dialog.set_bg_task_manager(self._task_manager)
note_dialog.set_current_entity(self._entity_type, self._entity_id)
Expand Down Expand Up @@ -824,22 +845,7 @@ def update_note_attachment_widget(self, note_id):
attachment_req["attachment_group_id"],
attachment_req["attachment_data"])

def _on_note_created(self, entity):
"""
Callback when an entity is created by an underlying widget.

:param entity: The Shotgun entity that was created.
"""
if entity["type"] != "Note":
# log error
return

self.rescan()
if self.notes_are_selectable:
self._select_on_arrival = entity
self.entity_created.emit(entity)

def create_note(self, data):
def create_note(self, data, userdata):
entity_id = self._entity_id
if entity_id is None:
self.engine.log_debug("Skipping New Note Creation in activity stream - No entity id.")
Expand All @@ -856,7 +862,7 @@ def create_note(self, data):
if data["project"] is None:
data["project"] = self._bundle.context.project

self._note_creator.submit(data)
self._note_creator.submit(data, userdata)

############################################################################
# internals
Expand Down Expand Up @@ -1201,7 +1207,14 @@ def _process_new_note(self, activity_id, note_id):
else:
self.note_arrived.emit(note_id)

def _on_entity_created(self, entity):
def _on_new_entity_requested_internally(self, entity_type, request_id):
"""
Called immediately when the user has initiated the creation of a new note,
before the shotgun API requests have been processed.
"""
self.new_entity_requested_internally.emit(entity_type, request_id)

def _on_entity_created(self, entity, request_id):
"""
Callback when an entity is created by an underlying widget.

Expand All @@ -1210,6 +1223,25 @@ def _on_entity_created(self, entity):
if self.notes_are_selectable and entity["type"] == "Note":
self._select_on_arrival = entity
self.entity_created.emit(entity)
self.entity_created_internally.emit(entity, request_id)

def _on_note_created_externally(self, entity, userdata=None):
"""
Callback when an entity is created by an underlying widget.

:param entity: The Shotgun entity that was created.
:param userdata: Optional userdata that may have been passed to the toolkit in a request
to create a note.
"""
if entity["type"] != "Note":
# log error
return

self.rescan()
if self.notes_are_selectable:
self._select_on_arrival = entity
self.entity_created.emit(entity)
self.note_created_externally.emit(entity, userdata)

def __send_note_content_to_cache_and_sg(self, sg, data):
"""
Expand Down
24 changes: 15 additions & 9 deletions python/note_input_widget/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ class NoteInputWidget(QtGui.QWidget):
replied to.
:signal close_clicked: Emitted if a user chooses to cancel the note
creation by clicking the X button.
:signal new_entity_requested: Emitted if the user chooses to create a new
Note or Reply entity. Can be used to perform application logic specific
to the moment when the user initiates the creation of the note.
:signal entity_created: Emitted when a Shotgun entity is created, which
will be either a Note or Reply entity, depending on situation. The
entity dictionary, as provided by the API, will be sent.
Expand All @@ -46,14 +49,18 @@ class NoteInputWidget(QtGui.QWidget):
data_updated = QtCore.Signal()
close_clicked = QtCore.Signal()

# Emitted when a new note or reply will be created. The creation request id
# is sent and will match the id sent with the entity_created signal.
# Todo: This has functional overlap with the pre_submit_callback...resolve that...
new_entity_requested = QtCore.Signal(str, int)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gomezll I added these extra signals and functionality before I realized there is already a mechanism that supplies similar functionality - the pre_submit_callback. Imo the existing callback is a bit odd since it passes the NoteInputWidget object as its parameter which feels a bit arbitrary, and some work would be needed to use that one since the pending request UID would need to be exposed through a function and that also seems a bit random.

I took the approach of adding new signals specific for the exact event type that we're trying to track (shotgun note creations and external user creations (in activity_stream.py), which seems nice and explicit but also makes things a bit busier.

Interested to see what you think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Your approach with new signals seems justified and appropriate to me Ed. Looks good


# Emitted when a Note or Reply entity is created. The
# entity type as a string and id as an int will be
# provided.
# provided along with the request id.
#
# dict(entity_type="Note", id=1234)
entity_created = QtCore.Signal(object)


entity_created = QtCore.Signal(object, int)

def __init__(self, parent):
"""
:param parent: The parent QWidget for this control
Expand Down Expand Up @@ -280,6 +287,9 @@ def _submit(self):
self._processing_id = self.__sg_data_retriever.execute_method(self._async_submit, data)
if self._outgoing_tasks:
self._outgoing_tasks.add(self._processing_id)
# If the entity being acted on is a Note then we are creating a Reply, otherwise a "Note"
new_entity_type = "Reply" if data["entity"]["type"] == "Note" else "Note"
self.new_entity_requested.emit(new_entity_type, int(self._processing_id))
else:
raise TankError("Please associate this class with a background task processor.")

Expand Down Expand Up @@ -575,7 +585,6 @@ def __upload_thumbnail(self, parent_entity, sg, data):
self.__upload_file(png_path, parent_entity, sg)
os.remove(png_path)


def __on_worker_failure(self, uid, msg):
"""
Asynchronous callback - the worker thread errored.
Expand All @@ -592,7 +601,6 @@ def __on_worker_failure(self, uid, msg):
QtGui.QMessageBox.critical(None, "Shotgun Error", msg)
if self._outgoing_tasks and self._outgoing_tasks.has(uid):
self._outgoing_tasks.remove(uid)


def __on_worker_signal(self, uid, request_type, data):
"""
Expand All @@ -612,13 +620,11 @@ def __on_worker_signal(self, uid, request_type, data):
self.clear()
self._bundle.log_debug("Update call complete! Return data: %s" % data)
self.data_updated.emit()
self.entity_created.emit(data["return_value"])
self.entity_created.emit(data["return_value"], int(uid))
if self._outgoing_tasks and self._outgoing_tasks.has(uid):
self._outgoing_tasks.remove(uid)
self.__overlay.hide()



def __format_thumbnail(self, pixmap_obj):
"""
Given a screengrab, create a thumbnail object, scaled to 96x75 px
Expand Down
58 changes: 49 additions & 9 deletions python/version_details/version_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ class VersionDetailsWidget(QtGui.QWidget):
QT Widget that displays details and Note thread data for a given Version
entity.

:signal entity_created(object): Fires when a Note or Reply entity is created by
:signal entity_created(object, object): Fires when a Note or Reply entity is created by
an underlying widget within the activity stream. Passes on a Shotgun
entity definition in the form of a dict.
entity definition in the form of a dict and optional userdata a requested note.
:signal entity_loaded(object): Fires when a Version entity has been loaded by
the widget. Passes on a Shotgun entity definition in the form of a dict.
:signal note_selected(int): Fires when a Note entity is selected in the widget's
Expand All @@ -82,10 +82,24 @@ class VersionDetailsWidget(QtGui.QWidget):
NOTE_MARKUP_PREFIX = "__note_markup__"
NOTE_THUMBNAIL_PREFIX = "__note_thumbnail__"

# Emitted when an entity is created by the panel. The
# Emitted when the user has initiated the creation of a new Note by interacting
# with this widget. Contains entity type and request_id.
new_entity_requested_internally = QtCore.Signal(str, int)

# Emitted when an entity is finished being created. The
# entity type as a string and id as an int are passed
# along.
# along, along with optional userdata. This is called both for entities created
# internally (originated within this widget), or via external requests to `create_note`.
entity_created = QtCore.Signal(object)
# Emitted when an entity is finished being created after being requested via
# a call to `create_note`. The entity type as a string and id as an int are passed
# along, along with optional userdata.
note_created_externally = QtCore.Signal(object, object)
# Emitted when an entity is finished being created after being requested via
# a call to `create_note`. The entity type as a string and id as an int are passed
# along, along with an id identifying the request, which will match the id emitted
# by `new_entity_requested_internally`.
entity_created_internally = QtCore.Signal(object, int)

# Emitted when an entity is loaded in the panel.
entity_loaded = QtCore.Signal(object)
Expand Down Expand Up @@ -274,9 +288,10 @@ def __init__(self, bg_task_manager, parent=None, entity=None):

# We will be passing up our own signal when note and reply entities
# are created.
self.ui.note_stream_widget.entity_created.connect(
self._entity_created,
)
self.ui.note_stream_widget.entity_created.connect(self._entity_created)
self.ui.note_stream_widget.new_entity_requested_internally.connect(self._new_entity_requested_internally)
self.ui.note_stream_widget.entity_created_internally.connect(self._entity_created_internally)
self.ui.note_stream_widget.note_created_externally.connect(self._note_created_externally)

self.load_data(entity)
self._load_stylesheet()
Expand Down Expand Up @@ -739,8 +754,8 @@ def use_styled_title_bar(self, dock_widget):
self.show_title_bar_buttons(True)
dock_widget.dockLocationChanged.connect(self._dock_location_changed)

def create_note(self, data):
self.ui.note_stream_widget.create_note(data)
def create_note(self, data, userdata):
self.ui.note_stream_widget.create_note(data, userdata)

def hide_note_widget(self, note_id):
"""
Expand Down Expand Up @@ -859,6 +874,31 @@ def _entity_created(self, entity):
"""
self.entity_created.emit(entity)

def _new_entity_requested_internally(self, entity_type, id):
"""
Emits the new_entity_requested_internally signal.

:param str entity_type: The Shotgun entity type that will be created.
:param int id: The unique id for this note creation command.
"""
self.new_entity_requested_internally.emit(entity_type, id)

def _entity_created_internally(self, entity, id):
"""
Emits the entity_created signal.

:param dict entity: The Shotgun entity dict that was created.
"""
self.entity_created_internally.emit(entity, id)

def _note_created_externally(self, entity, userdata):
"""
Emits the entity_created signal.

:param dict entity: The Shotgun entity dict that was created.
"""
self.note_created_externally.emit(entity, userdata)

def _field_menu_triggered(self, action):
"""
Adds or removes a field when it checked or unchecked
Expand Down