Conversation
ocawthorne
left a comment
ocawthorne
left a comment
There was a problem hiding this comment.
Just a few suggestions for improvement - I'll hold off on testing just in case you want to take on some of the more major suggestions detailed in the comments
bulk_reassign_docs.py
Outdated
| try: | ||
| response = client.patch( | ||
| f"v2/files/{file_id}", | ||
| json={"ownerId": user_id} |
There was a problem hiding this comment.
Without a parentId, what happens with workbooks and unsaved explorations? Would these all be dumped into someone's "My documents" section? (I assume if the workbooks are in a workspace, then they won't be moved)
Could it be better to allow the end user to optionally designate a folder into which these reassigned files will go?
There was a problem hiding this comment.
added optional arg for this
ocawthorne
left a comment
ocawthorne
left a comment
There was a problem hiding this comment.
LGTM - in summary, there's just a few minor camel-to-snake cases and validating that the destination folder is owned by the intended owner, but other than that, all clear (spare for testing) - will get ready to test soon.
bulk_reassign_docs.py
Outdated
|
|
||
| """ | ||
| updateFileBody={"ownerId": user_id} | ||
| if folderID: |
There was a problem hiding this comment.
We should probably also verify that the owner of the destination folder matches the new_owner_id
There was a problem hiding this comment.
We should probably throw an error and abort entirely if the owner of the folder doesn't match the new owner's ID actually
There was a problem hiding this comment.
after some back and forth on this, i think that we shouldnt need the folder owner to be match the person (what if i put it in a team folder for example). That said, I did validate that the ID is type folder
2 participants
No description provided.