Skip to content

Link to love recipients explore page after sending love. #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# This requirements file lists all dependecies for this project.
# Run 'make lib' to install these dependencies in this project's lib directory.
boto==2.22.0
dominate==2.3.1
Flask==0.10.1
Flask-Themes2==0.1.4
itsdangerous==0.24
Expand Down
26 changes: 26 additions & 0 deletions tests/util/markup_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# -*- coding: utf-8 -*-
import unittest

from testing.util import get_test_app
from util.markup import explore_links


class ExploreLinksTest(unittest.TestCase):
"""Tests util.email.explore_links()"""

def setUp(self):
self.app = get_test_app().app
self.app.config.update(SERVER_NAME='foo.com')
self.app_context = self.app.app_context()
self.app_context.push()

def tearDown(self):
self.app.config.update(SERVER_NAME=None)
self.app_context.pop()

def test_empty_list(self):
self.assertEqual(explore_links([], self.app_context), [])

def test_explore_links(self):
links = explore_links(['darwin'], self.app_context)
self.assertEqual(links, ['<a href="http://foo.com/explore?user=darwin">darwin</a>'])
5 changes: 5 additions & 0 deletions themes/default/static/css/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -269,3 +269,8 @@ h4 {
.pagination-result {
text-align: center;
}

.alert-success a {
color: #468847;
text-decoration: underline;
}
11 changes: 11 additions & 0 deletions util/markup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# -*- coding: utf-8 -*-
from dominate.tags import a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you plan on making more use of this in the future, e.g. for manipulating the DOM? It seems overkill to add the library just to generate <a href="...">...</a>.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. I can‘t say right now.

I just didn‘t feel like building tag(s) myself and re-implementing the wheel. And since this lib seems kind of maintained and has no other dependencies I felt okay adding it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are you reimplementing the wheel?

 a(employee, href=url_for('explore', user=employee)).render()
'<a href="{}">{}</a>'.format(url_for('explore', user=employee), employee)

I'm generally wary of adding these kinds of dependencies, it's just one more thing to maintain that adds bloat to our project. And it's not like a) we expect changes to how a tags are built that this package will cover for us; or b) that we not already have folders full of HTML. I consider this a general feeling of not wanting to put HTML markup in a "code" file, while the library just hides the fact that you do this.

from flask import url_for


def explore_links(employees, app_context):
with app_context:
return [
a(employee, href=url_for('explore', user=employee)).render()
for employee in employees
]
12 changes: 11 additions & 1 deletion views/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
from datetime import timedelta

from flask import abort
from flask import current_app
from flask import flash
from flask import redirect
from flask import request
from flask import url_for
from markupsafe import Markup

import logic.alias
import logic.department
Expand All @@ -34,6 +36,7 @@
from util.decorators import admin_required
from util.decorators import csrf_protect
from util.decorators import user_required
from util.markup import explore_links
from util.recipient import sanitize_recipients
from util.render import render_template
from views import common
Expand Down Expand Up @@ -214,7 +217,14 @@ def love():
try:
logic.love.send_loves(recipients, message, secret=secret)

flash('{}ove sent to {}!'.format('Secret l' if secret else 'L', recipients_display_str))
links = explore_links(recipients, current_app.app_context())
flash(
Markup('{initial}ove sent to {recipients}!'.format(
initial='Secret l' if secret else 'L',
recipients=', '.join(links),
))
)

return redirect(url_for('home'))
except TaintedLove as exc:
if exc.is_error:
Expand Down