Bug 1309227 - Improve form error handling and other UX things #37

jezdez · 2016-10-12T13:57:45Z

Bugs: https://bugzilla.mozilla.org/showdependencytree.cgi?id=1309227

This change is

jezdez · 2016-10-12T13:58:31Z

This is WIP, the jobs app is missing at the moment

codecov-io · 2016-10-12T13:59:46Z

Current coverage is 79.84% (diff: 84.01%)

Merging #37 into master will decrease coverage by 1.40%

@@             master        #37   diff @@
==========================================
  Files            35         34     -1   
  Lines           880        893    +13   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            715        713     -2   
- Misses          165        180    +15   
  Partials          0          0

Powered by Codecov. Last update 15fcde0...a7de05d

This also fixes tons of UI and UX issues like terminating clusters without confirmation

This also gets rid of client side form validation, adds client side notebook rendering of Spark job notebooks, improves the layout and markup of various views, and many other smaller fixes.

This gets rid of atmo.utils in favor of a flatter package layout. Renames the atmo.schedule module to atmo.jobs to prevent overlap with atmo.scheduling.

This prevented to edit a job

This also formats the dates as simplified ISO format since we assume UTC

Also reshuffle navbar links

maurodoglio

What a fantastic work 🙀
There are a few nits to fix, but this is as good as a tiramisú 🍰
rwc+

maurodoglio · 2016-10-17T05:16:05Z

atmo/clusters/forms.py

-        cluster.deactivate()
-        return cluster
+    def clean_confirmation(self):
+        confirmation = self.cleaned_data.get('confirmation', None)


None can be omitted, right?

maurodoglio · 2016-10-17T05:23:52Z

atmo/templates/atmo/cluster-detail.html

+      The notebook(s) will then show up in the Jupyter files list.
+    </p>
+    <p>
+      Now you can launch your Jupyter notebook on <a href="http://localhost:8888" target="_blank">localhost:8000</a>.


s/8000/8888

maurodoglio · 2016-10-17T06:15:49Z

atmo/templates/atmo/spark-job-detail.html

+    {% if job.notebook_content %}
+    <textarea id="notebook-content" class="hidden">{{ job.notebook_content }}</textarea>
+    {% else %}
+    Couldn't fetch Notebook content from S3


I would omit from s3.

maurodoglio · 2016-10-17T06:21:57Z

atmo/forms.py

@@ -5,13 +5,10 @@

 class PublicKeyFileField(forms.FileField):
    """
-    Custom Django file field that only accepts SSH public keys.
+    Custom Django for file field that only accepts SSH public keys.


s/Custom Django for// ?

maurodoglio · 2016-10-17T06:59:03Z

atmo/clusters/management/commands/delete_clusters.py

@@ -2,7 +2,7 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, you can obtain one at http://mozilla.org/MPL/2.0/.
 from django.core.management.base import BaseCommand
-from atmo.clusters.jobs import delete_clusters
+from ..jobs import delete_clusters


s/../.../ (or use abs import)
there are a few occurrences of this below

jezdez · 2016-10-17T08:40:04Z

Some current screenshots:

This prevent double submission and adds a reset button, too

maurodoglio

r+

Bumps [cryptiles](https://github.com/hapijs/cryptiles) from 3.1.2 to 3.1.4. **This update includes security fixes.** <details> <summary>Vulnerabilities fixed</summary> *Sourced from [The npm Advisory Database](https://npmjs.com/advisories/720).* > **Insufficient Entropy** > Versions of `cryptiles` from version 3.1.0 through 3.1.2, and versions 4.0.0 to version 4.1.1 are vulnerable to insufficient entropy. The `randomDigits` method generates digits that lack a perfect distribution over enough attempts. > > Affected versions: >=3.1.0 <3.1.3; >=4.0.0 <4.1.2 </details> <details> <summary>Commits</summary> - [`52f2f68`](hapijs/cryptiles@52f2f68) 3.1.4 - [`88d8409`](hapijs/cryptiles@88d8409) Remove engines. Closes [#37](https://github-redirect.dependabot.com/hapijs/cryptiles/issues/37) - [`2c3dcdf`](hapijs/cryptiles@2c3dcdf) node 10 globals - [`4340e43`](hapijs/cryptiles@4340e43) Drop node latest - [`7845877`](hapijs/cryptiles@7845877) Increase timeout - [`2d626ed`](hapijs/cryptiles@2d626ed) 3.1.3 - [`cb6bd64`](hapijs/cryptiles@cb6bd64) Backport fix [#34](https://github-redirect.dependabot.com/hapijs/cryptiles/issues/34). Closes [#35](https://github-redirect.dependabot.com/hapijs/cryptiles/issues/35) - See full diff in [compare view](hapijs/cryptiles@v3.1.2...v3.1.4) </details> <br /> [![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=cryptiles&package-manager=npm_and_yarn&previous-version=3.1.2&new-version=3.1.4)](https://dependabot.com/compatibility-score.html?dependency-name=cryptiles&package-manager=npm_and_yarn&previous-version=3.1.2&new-version=3.1.4) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot ignore this [patch|minor|major] version` will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language - `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com): - Update frequency (including time of day and day of week) - Automerge options (never/patch/minor, and dev/runtime dependencies) - Pull request limits (per update run and/or open at any time) - Out-of-range updates (receive only lockfile updates, if desired) - Security updates (receive only security updates, if desired) Finally, you can contact us by mentioning @dependabot. </details>

jezdez mentioned this pull request Oct 14, 2016

delete / edit buttons in user dash - not clear which job will be affected #39

Closed

jezdez added 20 commits October 14, 2016 14:08

Bug 1309227 - Port cluster forms to improved error handling

4c2bb4f

This also fixes tons of UI and UX issues like terminating clusters without confirmation

Fix bug 1309536 - Sort clusters by date

c9ee255

Fix cluster tests.

0e9be85

Improved jobs app

2d85b3a

This also gets rid of client side form validation, adds client side notebook rendering of Spark job notebooks, improves the layout and markup of various views, and many other smaller fixes.

Flatten the atmo package a bit

d4283a8

This gets rid of atmo.utils in favor of a flatter package layout. Renames the atmo.schedule module to atmo.jobs to prevent overlap with atmo.scheduling.

Add MPL statement to new files

36d80e7

Remove old form instance from dashboard view

c91aed0

Refactor schedule registry slightly

5718681

Add datetimepicker to jobs new and edit form

25dabc6

Use 24h based datepicket and prevent dates in the past

ab79c1f

Remove lower date limitaton

1846547

This prevented to edit a job

Spark job frontend fixes

b017c24

Refactor spark job forms to be DRY

ec8b591

Fix bug 1309545 - Horizontal dashboard sections

7151e23

This also formats the dates as simplified ISO format since we assume UTC

Fix bug 1309549 - Link to docs

2fa5c65

Fix bug 1309554 - Grey out terminate and edit button for clusters

9a077ca

Fix bug 1309572 - add note about times being in UTC to footer

6e2e53d

Also reshuffle navbar links

Fix bug 1309543 - Add filters for clusters in dashboard

1d1d5b1

Different grid

5f1f962

Fix bug 1309855 - Add accounting tags to AWS calls

a7de05d

jezdez force-pushed the bug1309227 branch from 17f98ed to a7de05d Compare October 14, 2016 12:08

maurodoglio suggested changes Oct 17, 2016

View reviewed changes

jezdez added 3 commits October 17, 2016 10:58

Add JS to disable for submission on submit

c60207d

This prevent double submission and adds a reset button, too

Apply review fixes

cf91a0f

Add EditorConfig

af0cbab

maurodoglio approved these changes Oct 17, 2016

View reviewed changes

jezdez merged commit 4209cd9 into master Oct 17, 2016

jezdez deleted the bug1309227 branch October 17, 2016 09:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1309227 - Improve form error handling and other UX things #37

Bug 1309227 - Improve form error handling and other UX things #37

jezdez commented Oct 12, 2016 •

edited

Loading

jezdez commented Oct 12, 2016

codecov-io commented Oct 12, 2016 •

edited

Loading

maurodoglio left a comment

maurodoglio Oct 17, 2016

jezdez Oct 17, 2016

maurodoglio Oct 17, 2016

jezdez Oct 17, 2016

maurodoglio Oct 17, 2016

jezdez Oct 17, 2016

maurodoglio Oct 17, 2016

maurodoglio Oct 17, 2016

jezdez Oct 17, 2016

jezdez commented Oct 17, 2016

maurodoglio left a comment

Bug 1309227 - Improve form error handling and other UX things #37

Bug 1309227 - Improve form error handling and other UX things #37

Conversation

jezdez commented Oct 12, 2016 • edited Loading

jezdez commented Oct 12, 2016

codecov-io commented Oct 12, 2016 • edited Loading

Current coverage is 79.84% (diff: 84.01%)

maurodoglio left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jezdez commented Oct 17, 2016

maurodoglio left a comment

Choose a reason for hiding this comment

jezdez commented Oct 12, 2016 •

edited

Loading

codecov-io commented Oct 12, 2016 •

edited

Loading