GUI scaffolding and core functions #29
Conversation
…hings in the right sort of places. Now iterate. :-)
|
Looking awesome. Btw a few of us ran into mu-editor/mu#502 (a cool project! ;)) while testing this out today, so heads up to other reviewers to run: and running the application here should occur without issue |
|
This PR is ready for review. I've added a bunch of generic code scaffolding for interacting with the remote API, I've also refactored the Happy to answer questions..! |
|
No idea why CI is failing. Help!?!?! |
|
Hm, yeah that is indeed a mysterious CI failure. Will SSH in and take a look... |
|
Ah, so it looks like the issue is occurring during the loading of resources: Will dig further |
|
Hey @ntoll, so - while I'm not sure why there is divergence between CI and the local dev env (all tests pass locally for me), the issue causing the fatal error here in CI is Instantiating a have you seen this before? |
securedrop_client/__init__.py
Outdated
| language_code = 'en' | ||
| # DEBUG/TRANSLATE: override the language code here (e.g. to Chinese). | ||
| # language_code = 'zh' | ||
| gettext.translation('mu', localedir=localedir, |
There was a problem hiding this comment.
mu -> securedrop_client
for the domain here
securedrop_client/storage.py
Outdated
| @@ -178,7 +214,7 @@ def find_or_create_user(username, session): | |||
| Returns a user object representing the referenced username. If the username | |||
| does not already exist in the data, a new instance is created. | |||
| """ | |||
| user = session.query(User).filter_by(username=username) | |||
| user = list(session.query(User).filter_by(username=username)) | |||
There was a problem hiding this comment.
I noticed the following about the sync logic when logging in using the client GUI: journalist users can change their journalist username so we should be using the journalist UUID as a unique identifier (journalist UUID will not change). Unfortunately the SDK doesn't expose this right now on its Reply objects (only the journalist username), so I made freedomofpress/securedrop-sdk#19 to resolve. Then we can do something like the following for the find_or_create_user logic:
diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py
index 59e9a9c..7e4ee28 100644
--- a/securedrop_client/storage.py
+++ b/securedrop_client/storage.py
@@ -187,7 +187,8 @@ def update_replies(remote_replies, local_replies, session):
if reply.uuid in local_uuids:
# Update an existing record.
local_reply = [r for r in local_replies if r.uuid == reply.uuid][0]
- user = find_or_create_user(reply.journalist_username, session)
+ user = find_or_create_user(reply.journalist_uuid,
+ reply.journalist_username, session)
local_reply.journalist_id = user.id
local_reply.filename = reply.filename
local_reply.size = reply.size
@@ -197,7 +198,8 @@ def update_replies(remote_replies, local_replies, session):
# A new reply to be added to the database.
source_uuid = reply.source_uuid
source = session.query(Source).filter_by(uuid=source_uuid)[0]
- user = find_or_create_user(reply.journalist_username, session)
+ user = find_or_create_user(reply.journalist_uuid,
+ reply.journalist_username, session)
nr = Reply(reply.uuid, user, source, reply.filename, reply.size)
session.add(nr)
logger.info('Added new reply {}'.format(reply.uuid))
@@ -209,15 +211,27 @@ def update_replies(remote_replies, local_replies, session):
session.commit()
-def find_or_create_user(username, session):
- """
- Returns a user object representing the referenced username. If the username
- does not already exist in the data, a new instance is created.
- """
- user = list(session.query(User).filter_by(username=username))
- if user:
- return user[0]
- new_user = User(username)
- session.add(new_user)
- session.commit()
- return new_user
+def find_or_create_user(uuid, username, session):
+ """
+ Returns a user object representing the referenced journalist UUID.
+ If the user does not already exist in the data, a new instance is created.
+ If the user exists but the username has changed, the username is updated.
+ """
+ user = session.query(User).filter_by(uuid=uuid).one()
+
+ if user and user.username == username:
+ # User exists in the local database and the username is unchanged.
+ return user
+ elif user and user.username != username:
+ # User exists in the local database but the username is changed.
+ user.username = username
+ session.add(user)
+ session.commit()
+ return user
+ else:
+ # User does not exist in the local database.
+ new_user = User(username)
+ new_user.uuid = uuid
+ session.add(new_user)
+ session.commit()
+ return new_user
Let me know if you notice an error with my logic here
|
@ntoll and I just had a quick chat about this PR, which is looking great! The final steps prior to merge are:
This gives us a nice foundation in |
ci: run tests also under python 3.7
Have CI test and build on bullseye too
This is currently a work in progress. This PR is simply a place for discussion and to give more visibility to the work being done to initially address #15 and #14.
Things to note:
app.py.Clientclass inlogic.py.guinamespace. All interaction with the UI is via theWindowclass ingui.main.gui.widgets. These should only be used by the afore mentionedWindowclass.resourcesnamespace containing some utility functions and CSS/image assets has been created.logicmodule for the rough draft which is, at this stage, very much a work in progress).Right now, it looks like this: