Page MenuHomePhabricator

display upload data
ClosedPublic

Authored by brooke on Jul 21 2019, 3:45 PM.

Details

Summary

display upload data and implement user input data type

@chris,

  1. data_management.py can render the drop down table I want
  2. simui_select_item_view.py is not functional right now but it is a starting point
  3. I am still working on extract line 32-36 from data_management.py into simui_select_item_view.py

Test Plan

code review

Diff Detail

Repository
rSIM Simplex
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

brooke requested review of this revision.Jul 21 2019, 3:45 PM
brooke created this revision.
brooke edited the summary of this revision. (Show Details)Jul 21 2019, 3:46 PM
chris added inline comments.Jul 21 2019, 3:49 PM
simplex/application/storage/data_management.py
16

I think type is a reserved keyword in base Python. E.g.,

>>> type()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: type() takes 1 or 3 arguments
>>> type
<class 'type'>

Do you mind renaming to something that isn't going to conflict with other classes/functions?

simplex/simplex.py
79

else : โ†’ else:

simplex/templates/manage/data.html
22

Could you switch to using the Markup(...) syntax? I think that gives us a marginally greater amount of control in terms of making sure that content is sanitized and escaped properly before being passed to Flask's HTML rendering engine

brooke added inline comments.Jul 21 2019, 3:53 PM
simplex/application/storage/data_management.py
16

oh yeah. good call out.

simplex/simplex.py
79

yep. update in next diff.

simplex/templates/manage/data.html
22

Agree. will do so.

brooke updated this revision to Diff 33.Jul 21 2019, 4:08 PM
  • fix some style issues
chris added a comment.Jul 21 2019, 4:13 PM

Couple more super minor comments, then you're good to go!

simplex/application/storage/data_management.py
21

Do you mind just fixing this PEP error really quickly? I think if you take the [ and just move it up to the end of line 20, Phab will stop yelling at you about this.

simplex/templates/manage/data.html
22

Since you've gotten the Markup() piece in place above, you can go ahead and remove the | safe from your code here and below in line 20

brooke updated this revision to Diff 37.Jul 21 2019, 8:12 PM
  • implement select data type
chris added a comment.Jul 21 2019, 8:14 PM

Could you move this most recent commit into its own code review? I think things are going to get too messy here otherwise. Thanks!

brooke edited the summary of this revision. (Show Details)Jul 21 2019, 8:16 PM
brooke edited the summary of this revision. (Show Details)
chris added a comment.Jul 24 2019, 7:07 PM
git reset --hard origin/dev
arc patch --diff 33
arc diff --update D13
git checkout -b tmp
arc patch --diff 37
arc diff HEAD^
git checkout dev
# wait for Chris to accept D13
arc land
git checkout tmp
git rebase dev
# wait for Chris to accept diff
arc land
brooke updated this revision to Diff 38.Jul 24 2019, 7:09 PM
brooke edited the summary of this revision. (Show Details)

arc diff --update D13

chris accepted this revision.Jul 24 2019, 7:10 PM
This revision is now accepted and ready to land.Jul 24 2019, 7:10 PM
brooke updated this revision to Diff 39.Jul 24 2019, 7:10 PM

implement select function

This revision was landed with ongoing or failed builds.Jul 24 2019, 7:11 PM
Closed by commit rSIMd9f88f25cd39: display upload data (authored by brooke). ยท Explain Why
This revision was automatically updated to reflect the committed changes.

after accept incoming changes, I did
git add --all
git commit -m "rebase"
git rebase --continue

Applying: display upload data
No changes - did you forget to use 'git add'?
If there is nothing left to stage, chances are that something else
already introduced the same changes; you might want to skip this patch.

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".
chris added a comment.Jul 24 2019, 7:22 PM

weird. Just git rebase --skip

$ arc land
Landing current branch 'tmp'.
TARGET Landing onto "dev", selected by "arc.land.onto.default" configuration.
REMOTE Using remote "origin", the default remote under git.
FETCH Fetching origin/dev...
Authenticated to phabricator.tbmh.org ([50.116.49.41]:2222).
Transferred: sent 2812, received 3052 bytes, in 0.6 seconds
Bytes per second: sent 4425.4, received 4803.1
This commit will be landed:

  • a494ef1 rebase

Usage Exception: arc can not identify which revision exists on branch 'tmp'. Update the revision with recent changes to synchronize the branch name and hashes, or use 'arc amend' to amend the commit message at HEAD, or use '--revision <id>' to select a revision explicitly.

chris added a comment.Jul 24 2019, 7:27 PM
In D13#297, @brooke wrote:

$ arc land
Landing current branch 'tmp'.
TARGET Landing onto "dev", selected by "arc.land.onto.default" configuration.
REMOTE Using remote "origin", the default remote under git.
FETCH Fetching origin/dev...
Authenticated to phabricator.tbmh.org ([50.116.49.41]:2222).
Transferred: sent 2812, received 3052 bytes, in 0.6 seconds
Bytes per second: sent 4425.4, received 4803.1
This commit will be landed:

  • a494ef1 rebase

Usage Exception: arc can not identify which revision exists on branch 'tmp'. Update the revision with recent changes to synchronize the branch name and hashes, or use 'arc amend' to amend the commit message at HEAD, or use '--revision <id>' to select a revision explicitly.

I think this just became a Sunday problem ๐Ÿ˜œ

hahaha~ sorry! I was going to do something productive tonight. I am just so awesome!