Initial PR (complete layout tool) #14
Conversation
This orphan branch marks the the clean slate for a new in-toto layout creation wizard, which will be based on UI mockups: https://github.com/in-toto/layout-web-tool/tree/editor-and-wizard-wip/mockups Isolated snippets and ideas from the pure layout editor (WIP) https://github.com/in-toto/layout-web-tool/tree/develop and the basic wizard plus editor (WIP) https://github.com/in-toto/layout-web-tool/tree/editor-and-wizard-wip might end up in this branch. If we want to have a single flask app with a wizard and a editor module, we should consider using blueprints: http://flask.pocoo.org/docs/0.12/blueprints/ This commit provides some basic Flask web scaffolding.
An option group consists of an image (e.g. a logo), a name and an input type (radio or checkbox depending on whether multiple options are possible or not). If a user picks an option a form will be opened to add additional information for that option. An option can be for example a particular version control system, or a build command. The opened form will be pre-populated with the appropriate command. The user can then customize or change the command associated with that option.
SASS (Syntactically Awesome Stylesheets)[1] is a CSS preprocessor which makes writing CSS a lot easier, especially when including frontend frameworks (e.g. Bootstrap). This commit - replaces the main .css file with an empty .scss file, which will be compiled into .css (plus source map -> your browser's developer tools will need them). - adds the python scss compiler (libsass) to requirements.txt - adds a flask middleware to wizard.py to re-compile scss on each request. Alternatively, you can run a sass watcher in the project root, which will automatically compile on change. This is very useful when mapping files in your browser's developer console -> no need to refresh: ``` gem install sass sass --watch static/scss/main.scss:static/css/main.scss.css ``` [1] http://sass-lang.com/
- Adds package.json to install Bootstrap 4 (alpha) (use `npm install`) - Adds Boostrap HTML scaffolding - Import Boostrap scss sources in main scss (we'll probably remove some of them later) - Adds JS links to CDN (tempfix -> need to find a way to import them from node_modules, e.g. using `gulp`) - Adds node_modules to .gitignore
- Adds dummy opt-rows to see how this will look like. - Adds style for recess effect (this is where the forms will go), effect mostly taken from boostrap 3's "well". - Updates forms to align with bootstrap 4
- Removes topmost horizontal ruler and adds margin to progress bar - Removes links inheritance block (might be re-added after UI proto- typing) - Exchanges slim jquery with regular jquery CDN link (for fadeIn)
Basic versioning page consists of a grid for predefined options (currently dummy options) with radio buttons and a form (input field) that expands below the option. Additionally, there is a button to add a custom VCS commands (currently not limited to one - this should probably be changed). Expand/collapse and show/hide is performed using custom JS. The look and feel for this page probably won't change a lot but the code structure could be more elegant, (e.g. not one .row per "row"). Also I went back to style using bootstrap classes in the HTML instead of extending custom classes in SCSS - while this makes the HTML more convoluted, it is easier for me during prototyping (trying out bootstrap classes).
Adds a modal (overlay) that appears when clicking next on the versioning page and asks for a consent to clone the repo.
Creates new partials directory for templates that will be used repeatedly and adds partial with dummy versioning code.
As announced in 433f469 this commit cleans up the code structure of the options grid. There are no visible changes in the UI but the HTML and CSS looks like it should now. Changes: All cells are in a single `.row` div and the amount of cells actually visible on a single row depends on an SCSS constant (one per screen width breakpoint). Furthermore, each form is a direct child of an option cell and the effect of showing it in the subsequent is controlled using SCSS (above all media queries). Kudos to mix(in)-master-@pooledge and his awesome styling skills.
Macros are more flexible than included templates in terms of passing variables and nesting. This commit adds two macros one for the option grid and one for the custom options, both used in several pages of the wizard. Small variations between macro calling templates are handled using keyword arguments, whereas bigger variations (e.g. the expanded form contents) are handled using the macro call syntax [1]. Note: the call blocks themselves could be DRYed using markups. [1] http://jinja.pocoo.org/docs/2.9/templates/#call
- fixes indentation - removes comment - adds empty footer div, for now just for the margin, later add content
Boostrap's `collapse` shows error messages in the console, now uses tailor-made Jquery `slideUp`, `slideDown`, `slideToggle` instead. Also, replaces-id based targeting with relation-based, i.e. Click on `.opt-content` opens `.opt-form-cont` that has the same parent `.opt-cell`, which is more flexible.
The building page uses `option_grid` and `option_custom` macros with form call blocks, slighly different from versioning page.
`libsass` was used to compile *.scss to *.css on each request. Using the ruby `sass` cli tool with the `--watch` option instead to auto compile css on file change makes `libsass` obsolete. sass watch command, run in project root ``` sass --watch static/scss/main.scss:static/css/main.scss.css ```
All required vendor js files are installed to a local node_modules dir when running `npm install` in the root of the project. Instead of serving those assets directly from node_modules (or from a CDN as we used to), we use gulp.js[1] to copy them to a place where flask expects, i.e. the ./static/ dir. Note: Later we could add `gulp` tasks to concatenate and/or minify or uglify the assets on copy. Furthermore, this commit - adds installation instructions and development tips to README.md - and adds a html5 drag and drop sorting JS library to package.json [1] http://gulpjs.com/
Since there will be multiple building steps, the selected options should be sortable here. Todo: - Hook the sort container up with the options below, i.e. add/ remove on select/deselect - Think about position of the sorting container
Jinja has a handy lispum function http://jinja.pocoo.org/docs/2.9/templates/#lipsum
The QA page is similarly structured like VCS and building (without a sorting section), with a grid of predefined QA options with expandable forms and a section to add custom commands. Since the form is more complex than in the previous pages this commit also introduces a page internal macro for the form for DRY in predefined and custom option forms.
Similar like previous pages, with an option grid and a section to add custom options.
- Adds body background - Adds container borders - Enables box shadow for form inset effect
Use gulp instead (c.f. commit message in 2043387)
Adds margin between container and header and top-border
Adds D3.js as npm dependency and updates gulpfile to copy it into vendor dir on `gulp`. D3.js will be used to render in-toto layouts as graphs, a.o. on the software supply chain page. This commit also reads a demo layout (Note: layout not checked into VCS) in the `software_supply_chain` view and passes it as json string to the template, where it's assigned to a global JS variable later used to init the D3 SVG.`
The svg element will hold the software supply chain graphs. I use a trick [1] to make the svg and it's elements responsive (scalable). - Also generalizes inset style used for various elements, e.g. form, sortable and svg container [1] http://thenewcode.com/744/Make-SVG-Responsive
- JS data transformation that turns in-toto layout into data suitable for D3 graphs (will probably move transformation to server) - A mostly copy pasted, not yet working D3 froce directed graph (will probably use a different library, e.g. dagre-d3)
- Add newlines at end of file - Update project website URL in readme - Add note about `service` deprecation to readme - pin in-toto repo to commit in requirements file - git rename reverse_layout to create_layout - remove obsolete stand-alone script parts from create_layout - update create_layout header to show demo usage - update comment about message show duration in main.js - rename authorizing.html to authorizing_functionaries.html - make constant in uppercase in tooldb - Remove sample wsgi script. (The readme still has a link to a sample wsgi on the Flask website)
| @@ -0,0 +1,21 @@ | |||
| The MIT License (MIT) | |||
|
|
|||
| Copyright (c) 2017 New York University | |||
There was a problem hiding this comment.
👍
This looks like a standard MIT license, and copyright is assigned to New York University as requested by @JustinCappos
| @@ -0,0 +1,21 @@ | |||
| # Byte-compiled / optimized / DLL files | |||
There was a problem hiding this comment.
Since this is a Flask app, what about ignoring Flask-specific files?
Example:
instance/
.webassets-cache
There was a problem hiding this comment.
Added instance/.
Adding .webassets-cache won't be necessary as we don't use webassets/Flask-Assets
This is also what the extension `Flask-Session` uses for session ids.
As pointed out by @awwad the suggested snippet did not work. This commit fixes the snippet. The command line tool's usage message will be fixed with in-toto/in-toto#118
README.md
Outdated
| # in-toto Layout Creation Wizard | ||
|
|
||
| A Flask based web app to guide project owners through creating an | ||
| [in-toto layout](https://in-toto.io). |
There was a problem hiding this comment.
The linked text is in-toto layout, so I was expecting a link to something about in-toto's layout tool.
Maybe linking to https://in-toto.engineering.nyu.edu/, where an instance of the layout tool exists, makes more sense? This repository is also more about the layout tool rather than in-toto.
| @@ -0,0 +1,55 @@ | |||
| # in-toto Layout Creation Wizard | |||
There was a problem hiding this comment.
Consider saying somewhere in the README that an instance of the layout tool is available and/or link to it.
README.md
Outdated
| ### Installation | ||
|
|
||
| **Requirements** | ||
| - [Python 2.7](https://www.python.org/download/releases/2.7/) -- backend |
There was a problem hiding this comment.
Do you have to install any Python development headers?
Thanks to @awwad for rephrasing this. #14 (comment)
| sass --watch static/scss/main.scss:static/css/main.scss.css | ||
| ``` | ||
| - Make extensive use of (e.g. chrome's) browser developer tools, e.g. [map | ||
| DevTool files to your local workspace](https://developers.google.com/web/tools/setup/setup-workflow) to live edit `*.scss` and `*.js` files. |
There was a problem hiding this comment.
Does this project need an acknowledgement section?
| <div class="opt-row row no-gutters"> | ||
| <div class="opt-cell col opt-cell-top"> | ||
| <div class="opt-content"> | ||
| <div class="opt-text text-center">Custom Command</div> |
There was a problem hiding this comment.
The Custom Command field is listed before the other popular version control systems. Which option is the user likely to select? If it's not Custom Command, then it should be listed after the other popular options.
Before clicking any of the buttons, I thought the user was expected to enter the custom command and then select an icon.
What if instead of a long button for the custom command (which looks strange to me and is different in style from the other buttons) you provide an "other" button (of the same size as the others)?
There was a problem hiding this comment.
The idea was to make the "custom command" button easily recognizable among all the other buttons. But anther user has suggested something very similar, so I will probably change this. :)
There was a problem hiding this comment.
Will be fixed with #25 (comment)
| {%- endmacro %} | ||
|
|
||
| {% macro comment_form(value) %} | ||
| <h2>Doing things differently?</h2> |
There was a problem hiding this comment.
I'm not sure what this means. Is it asking if the user has a different supply chain procedure that the layout tool doesn't currently support?
Maybe the comment below the question ("Please leave us a comment and we will try to make this tool better") should say that the in-toto devs are open to the possibility of adding support for the user's uncommon procedure.
There was a problem hiding this comment.
Correct, this form is meant for feedback/complaints and the like. Do you have a suggestion for a different title/text?
Also, I guess right now this section has a too prominent place on every page. I can see how that could be confusing. Maybe we should replace it with a link to a contact form/mailto? What do you think?
| {% block content %} | ||
| {{ macros.progress_bar(value=30) }} | ||
| <hr> | ||
| <h1>How do you build your software?</h1> |
There was a problem hiding this comment.
How do Python projects build their software?
I was looking for a Python option (is it python setup.py sdist/bdist ?) and was surprised it wasn't listed, considering in-toto is written in Python. Or is that a different step, the distribution step? Maybe you are using the strict definition of a software build, and the tool only considers compiled software?
There was a problem hiding this comment.
Don't mean to be pedantic, but python setup.py sdist/bdist is packaging. setup.py build is closer to building, but there's still a lot of debate of if its really building. There is a whole rabbit hole you can go down if you read on it in the case of reproducible builds.
There was a problem hiding this comment.
It can probably be classified as packaging, but it's slightly complicated because the standard Python packaging tools combine the build and packaging steps. A bunch of files are added to a build/ directory when you run python setup.py bdist. The packaging user guides/documentation even use the word "building"
https://setuptools.readthedocs.io/en/latest/setuptools.html#developer-s-guide
And I think the Python community is working to separate the build and distribution steps judging from discussions on the Distutils forum. Maybe in-toto doesn't need to worry about the distinction, or just define "build" (if it hasn't already).
| @@ -0,0 +1,83 @@ | |||
| {#-################################################################# | |||
There was a problem hiding this comment.
The text and icons look a bit small on my screen, although I am on a MacBook with a 12-inch display.
There was a problem hiding this comment.
Will be considered in #2
| </script> | ||
| {{ macros.progress_bar(value=40) }} | ||
| <hr> | ||
| <h1>How do you assure your software's quality?</h1> |
There was a problem hiding this comment.
Is it possible for a project to use multiple tools, like unittest, doctest, and Travis CI. It seems that you can only select one icon.
There was a problem hiding this comment.
Yep, that's the idea of the apparently rather ambiguous "Add and continue on this page" button. :)
This problem is already documented in #25 (comment). I will try to come up with a better solution.
| <div class="functionary dz-container p-3 mt-2 {{'template' if template}}"> | ||
| <div class="m-2"> | ||
| <span class="functionary-name">{{name}}</span><br> | ||
| <small>(Drop public key or click in box to upload)</small> |
There was a problem hiding this comment.
I attempted to upload a public key, and it might have been an unsupported format because I got a popup error. I don't know what the error was because the window disappeared after 1-3 seconds.
| {% block content %} | ||
| {{ macros.progress_bar(value=75) }} | ||
| <hr> | ||
| <h1>Who is authorized to do what in your Project?</h1> |
There was a problem hiding this comment.
What am I supposed to enter in these input fields? The label says,
"authorize functionaries for this step in select box below." Maybe include an example. When I click on the input field I get, "No results found." Maybe it's because my upload of the public key failed...
| is allowed to do what in your project. The actors that are allowed to perform | ||
| a step or multiple steps in your software supply chain are called | ||
| <i>functionaries</i> and are identified by their respective public key. | ||
| Below you can define your functionaries and upload their public key.</p> |
There was a problem hiding this comment.
Consider stating which key formats are supported.
wizard.py
Outdated
| Views: | ||
| Each view is an entry point for an HTTP request (c.f. paths in @app.route | ||
| decorator). Most views correspond to a page the user surfs to and | ||
| therefor render and return the according template on a GET request. |
There was a problem hiding this comment.
nitpick: I've seen this a few times now, so I might as well mention it. I think the word you want to use here is corresponding.
"therefore render and return the corresponding template on a GET request."
wizard.py
Outdated
|
|
||
| @app.route("/versioning", methods=["GET", "POST"]) | ||
| @with_session_id | ||
| def versioning(): |
There was a problem hiding this comment.
Not a big deal, but at first I thought this was a page about versioning software. What about just vcs, it's concise and more obvious.
| functionary_name = request.form.get("functionary_name", None) | ||
|
|
||
| if not functionary_name: | ||
| flash("Something went wrong: We don't know which functionary," |
There was a problem hiding this comment.
Is there a better way to display errors? I'm not sure if a flash is an ideal approach.
There was a problem hiding this comment.
I suppose you and @aaaaalbert don't like that messages disappear after a certain time? This is not related to Flask's Message Flashing.
I will fix user feedback with #21
As pointed out by @SantiagoTorres we should check that the arguments to `form_data_to_ssc` have the same length, also inside the function. This commit adds the necessary if statements.
| flash("Something went wrong: We don't know which functionary," | ||
| " this key belongs to", "alert-danger") | ||
|
|
||
| return jsonify({"error": True}) |
There was a problem hiding this comment.
Do you want to return more specific info about the error?
There was a problem hiding this comment.
A better error message than "Something went wrong: We don't know which functionary, this key belongs to" or a specific error code? Do you have something in mind? Btw. this error should only occur if the user tampers with the form or the JS.
See http://flask.pocoo.org/docs/0.12/config/#instance-folders for more infos about flask's instance folders.
- Rephrase link to project website - Add link to deployed beta version - Mention python development headers - Add Acknowledgements section
lukpueh
force-pushed
the
wizard-beta
branch
from
ace0aeb to
855e5cc
Compare
Replace "therefor" with "therefore" and "according" with "corresponding" where necessary.
As @vladimir-v-diaz pointed out correctly vcs is more accurate in this context.
|
Thanks for the thorough mixed code review and user testing comments. Some of your comments I addressed directly and others I incorporated into existing and new issues. Please let me know if you feel the code is ready to be merged. |
This PR contains an entire Flask-based web app, used to serve a website that guides through the creation of an in-toto layout.
A deployed instance of the web-app can be found at in-toto.engineering.nyu.edu.
Please pardon the humongous PR, but given the workflow for this project it wouldn't have made a lot of sense to incrementally submit PRs. I thought about chopping up the diff to artificially create smaller-sized, themed PRs, but I guess that's not really worth the effort and just messes up the commit history.
Consider reviewing file by file. It does not have to be a single person who reviews everything. Just pick a group of files from below and notify the others that you are reviewing. And don't hesitate to ask me if anything is unclear.
Before starting to review, take a quick look at the issue tracker and also consider comments in the file headers and inline
FIXMEs/TODOs. There is no need to point out issues that I'm already aware of.Readme
Please review for completeness and conciseness. It would be great if you could
also try to set up the app locally using this Readme.
Monolithic back-end script
Contains all back-end code. Please review thoroughly!
Monolithic front-end script
Contains all non-third-party front-end code (mostly UI-related). Please review thoroughly!
Standalone layout creation module
A module used by the back-end script (view:
download_layout) to create the actual in-toto layout.Please review thoroughly!
Front-end content and style
A quick look at these files should be enough
Front- and back-end dependency management
A quick look at these files should be enough
Miscellaneous
A quick look at these files should be enough