DRY out flashed templates #1602
Labels
Comments
psivesely
added
UX
journalist interface (JI)
app
help wanted!
and removed
journalist interface (JI)
UX
labels
psivesely
pushed a commit
that referenced
this issue
Mar 14, 2017
* If a user tries to change a password for an account and enters the existing password, the operation is now idempotent. * The `/admin/edit/<int:user_id>` and `/account` routes now use the same template. Since the former provides a superset of the functionality of the latter, and there is no security risk in doing so, DRYing out the templates here made sense. While this fixes #1597, combining the templates also has potential to expose elements meant for admins to users in some future regression. To defend against this, I introduced a new functional test module, `tests.functional.make_account_changes`, which provides comprehensive checks to this regard. I also did some DRYing of these route functions since they both change passwords, commit account changes to the database, and flash messages regarding the success or failure of these operations. * I also made some improvements to the relevant flashed messages for account changes, making progress towards #1476, #1601, #1602. It seemed like "success" and "failure" were better categorical indicators than "notification" and "failure," so I went with that and used the green checkmark PNG we have already employed in the source interface. I changed all the flashed icons here to PNGs while I was at it to make a small step towards making the JI security-slider-friendly, and explicitly specified their sizes to make sure they rendered correctly. * Changing some of the flashed messages in the process of my refactor, I had to change the corresponding unit tests as well. To test both the content and category of the flashed message, I decided to take advantage of Flask-Testing's `assertMessageFlashed` method. This required installing an additional testing dependency, `blinker`, which I did without changing any other package versions.
psivesely
pushed a commit
that referenced
this issue
Mar 14, 2017
* If a user tries to change a password for an account and enters the existing password, the operation is now idempotent. Similarly, if an admin "changes" a user's username to itself, this operation is also idempotent. The new `commit_account_changes` function only writes to the database if the `db.User` object of the user's account being changed has been locally modified, which it determines via the `Session.is_modified(db.User)` method. * The `/admin/edit/<int:user_id>` and `/account` routes now use the same template. Since the former provides a superset of the functionality of the latter, and there is no security risk in doing so, DRYing out the templates here made sense. While this fixes #1597, combining the templates also has potential to expose elements meant for admins to users in some future regression. To defend against this, I introduced a new functional test module, `tests.functional.make_account_changes`, which provides comprehensive checks to this regard. I also did some DRYing of these route functions since they both change passwords, commit account changes to the database, and flash messages regarding the success or failure of these operations. * I also made some improvements to the relevant flashed messages for account changes, making progress towards #1476, #1601, #1602. It seemed like "success" and "failure" were better categorical indicators than "notification" and "failure," so I went with that and used the green checkmark PNG we have already employed in the source interface. I changed all the flashed icons here to PNGs while I was at it to make a small step towards making the JI security-slider-friendly, and explicitly specified their sizes to make sure they rendered correctly. * Changing some of the flashed messages in the process of my refactor, I had to change the corresponding unit tests as well. To test both the content and category of the flashed message, I decided to take advantage of Flask-Testing's `assertMessageFlashed` method. This required installing an additional testing dependency, `blinker`, which I did without changing any other package versions.
psivesely
pushed a commit
that referenced
this issue
Mar 22, 2017
* If a user tries to change a password for an account and enters the existing password, the operation is now idempotent. Similarly, if an admin "changes" a user's username to itself, this operation is also idempotent. The new `commit_account_changes` function only writes to the database if the `db.User` object of the user's account being changed has been locally modified, which it determines via the `Session.is_modified(db.User)` method. * The `/admin/edit/<int:user_id>` and `/account` routes now use the same template. Since the former provides a superset of the functionality of the latter, and there is no security risk in doing so, DRYing out the templates here made sense. While this fixes #1597, combining the templates also has potential to expose elements meant for admins to users in some future regression. To defend against this, I introduced a new functional test module, `tests.functional.make_account_changes`, which provides comprehensive checks to this regard. I also did some DRYing of these route functions since they both change passwords, commit account changes to the database, and flash messages regarding the success or failure of these operations. * I also made some improvements to the relevant flashed messages for account changes, making progress towards #1476, #1601, #1602. It seemed like "success" and "failure" were better categorical indicators than "notification" and "failure," so I went with that and used the green checkmark PNG we have already employed in the source interface. I changed all the flashed icons here to PNGs while I was at it to make a small step towards making the JI security-slider-friendly, and explicitly specified their sizes to make sure they rendered correctly. * Changing some of the flashed messages in the process of my refactor, I had to change the corresponding unit tests as well. To test both the content and category of the flashed message, I decided to take advantage of Flask-Testing's `assertMessageFlashed` method. This required installing an additional testing dependency, `blinker`, which I did without changing any other package versions.
psivesely
pushed a commit
that referenced
this issue
Apr 6, 2017
* If a user tries to change a password for an account and enters the existing password, the operation is now idempotent. Similarly, if an admin "changes" a user's username to itself, this operation is also idempotent. The new `commit_account_changes` function only writes to the database if the `db.User` object of the user's account being changed has been locally modified, which it determines via the `Session.is_modified(db.User)` method. * The `/admin/edit/<int:user_id>` and `/account` routes now use the same template. Since the former provides a superset of the functionality of the latter, and there is no security risk in doing so, DRYing out the templates here made sense. While this fixes #1597, combining the templates also has potential to expose elements meant for admins to users in some future regression. To defend against this, I introduced a new functional test module, `tests.functional.make_account_changes`, which provides comprehensive checks to this regard. I also did some DRYing of these route functions since they both change passwords, commit account changes to the database, and flash messages regarding the success or failure of these operations. * I also made some improvements to the relevant flashed messages for account changes, making progress towards #1476, #1601, #1602. It seemed like "success" and "failure" were better categorical indicators than "notification" and "failure," so I went with that and used the green checkmark PNG we have already employed in the source interface. I changed all the flashed icons here to PNGs while I was at it to make a small step towards making the JI security-slider-friendly, and explicitly specified their sizes to make sure they rendered correctly. * Changing some of the flashed messages in the process of my refactor, I had to change the corresponding unit tests as well. To test both the content and category of the flashed message, I decided to take advantage of Flask-Testing's `assertMessageFlashed` method. This required installing an additional testing dependency, `blinker`, which I did without changing any other package versions.
psivesely
pushed a commit
that referenced
this issue
Apr 6, 2017
* If a user tries to change a password for an account and enters the existing password, the operation is now idempotent. Similarly, if an admin "changes" a user's username to itself, this operation is also idempotent. The new `commit_account_changes` function only writes to the database if the `db.User` object of the user's account being changed has been locally modified, which it determines via the `Session.is_modified(db.User)` method. * The `/admin/edit/<int:user_id>` and `/account` routes now use the same template. Since the former provides a superset of the functionality of the latter, and there is no security risk in doing so, DRYing out the templates here made sense. While this fixes #1597, combining the templates also has potential to expose elements meant for admins to users in some future regression. To defend against this, I introduced a new functional test module, `tests.functional.make_account_changes`, which provides comprehensive checks to this regard. I also did some DRYing of these route functions since they both change passwords, commit account changes to the database, and flash messages regarding the success or failure of these operations. * I also made some improvements to the relevant flashed messages for account changes, making progress towards #1476, #1601, #1602. It seemed like "success" and "failure" were better categorical indicators than "notification" and "failure," so I went with that and used the green checkmark PNG we have already employed in the source interface. I changed all the flashed icons here to PNGs while I was at it to make a small step towards making the JI security-slider-friendly, and explicitly specified their sizes to make sure they rendered correctly. * Changing some of the flashed messages in the process of my refactor, I had to change the corresponding unit tests as well. To test both the content and category of the flashed message, I decided to take advantage of Flask-Testing's `assertMessageFlashed` method. This required installing an additional testing dependency, `blinker`, which I did without changing any other package versions.
redshiftzero
pushed a commit
that referenced
this issue
Apr 20, 2017
* If a user tries to change a password for an account and enters the existing password, the operation is now idempotent. Similarly, if an admin "changes" a user's username to itself, this operation is also idempotent. The new `commit_account_changes` function only writes to the database if the `db.User` object of the user's account being changed has been locally modified, which it determines via the `Session.is_modified(db.User)` method. * The `/admin/edit/<int:user_id>` and `/account` routes now use the same template. Since the former provides a superset of the functionality of the latter, and there is no security risk in doing so, DRYing out the templates here made sense. While this fixes #1597, combining the templates also has potential to expose elements meant for admins to users in some future regression. To defend against this, I introduced a new functional test module, `tests.functional.make_account_changes`, which provides comprehensive checks to this regard. I also did some DRYing of these route functions since they both change passwords, commit account changes to the database, and flash messages regarding the success or failure of these operations. * I also made some improvements to the relevant flashed messages for account changes, making progress towards #1476, #1601, #1602. It seemed like "success" and "failure" were better categorical indicators than "notification" and "failure," so I went with that and used the green checkmark PNG we have already employed in the source interface. I changed all the flashed icons here to PNGs while I was at it to make a small step towards making the JI security-slider-friendly, and explicitly specified their sizes to make sure they rendered correctly. * Changing some of the flashed messages in the process of my refactor, I had to change the corresponding unit tests as well. To test both the content and category of the flashed message, I decided to take advantage of Flask-Testing's `assertMessageFlashed` method. This required installing an additional testing dependency, `blinker`, which I did without changing any other package versions.
redshiftzero
pushed a commit
that referenced
this issue
Apr 20, 2017
* If a user tries to change a password for an account and enters the existing password, the operation is now idempotent. Similarly, if an admin "changes" a user's username to itself, this operation is also idempotent. The new `commit_account_changes` function only writes to the database if the `db.User` object of the user's account being changed has been locally modified, which it determines via the `Session.is_modified(db.User)` method. * The `/admin/edit/<int:user_id>` and `/account` routes now use the same template. Since the former provides a superset of the functionality of the latter, and there is no security risk in doing so, DRYing out the templates here made sense. While this fixes #1597, combining the templates also has potential to expose elements meant for admins to users in some future regression. To defend against this, I introduced a new functional test module, `tests.functional.make_account_changes`, which provides comprehensive checks to this regard. I also did some DRYing of these route functions since they both change passwords, commit account changes to the database, and flash messages regarding the success or failure of these operations. * I also made some improvements to the relevant flashed messages for account changes, making progress towards #1476, #1601, #1602. It seemed like "success" and "failure" were better categorical indicators than "notification" and "failure," so I went with that and used the green checkmark PNG we have already employed in the source interface. I changed all the flashed icons here to PNGs while I was at it to make a small step towards making the JI security-slider-friendly, and explicitly specified their sizes to make sure they rendered correctly. * Changing some of the flashed messages in the process of my refactor, I had to change the corresponding unit tests as well. To test both the content and category of the flashed message, I decided to take advantage of Flask-Testing's `assertMessageFlashed` method. This required installing an additional testing dependency, `blinker`, which I did without changing any other package versions.
|
imho having one template with lots of conditionals is less clear, so closing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Using
categoryargument offlask.flashwe can create a single template for each type of action, that is adapted based on the category argument (see the relevant Flask docs). E.g., for SI submissions, we can use a single template that has conditionals for possiblecategoryargumentsfirst_success,success, andfailure.The text was updated successfully, but these errors were encountered: