Privacy features && social handles #242
Conversation
|
Request from Katherine to remove Snapchat feature bc of sus connotations. Just Instagram for now |
| @@ -50,7 +50,8 @@ def api_students(): | |||
| if not criteria['filters'].get('school_code'): | |||
| criteria['filters']['school_code'] = [] | |||
| criteria['filters']['school_code'].append('YC') | |||
| students = Person.search(criteria) | |||
| students = Person.search_respect_privacy_include_persistent(criteria) | |||
There was a problem hiding this comment.
This function name is way too long
| @@ -1,9 +1,11 @@ | |||
| from app import app, db | |||
| from app.search import SearchableMixin | |||
| from app.util import get_now | |||
| from app.util import get_now, PERSISTENT_FIELDS, scrub_hidden_data | |||
There was a problem hiding this comment.
Hmmm I don't know how I feel about putting PERSISTENT_FIELDS under util... seems pretty specific to one file. Maybe make this a class variable under PersonPersistent?
| socials_snapchat = db.Column(db.String) | ||
|
|
||
| # privacy | ||
| privacy_hide_image = db.Column(db.Boolean) |
There was a problem hiding this comment.
I fundamentally disagree that we should keep track of what things people have marked as removed. It adds a ton of logic to our operations and risks giving people a totally false sense of security, since many people will stop with Yalies and not actually prevent their data from being accessed in any meaningful way. Instead, I think we should enable users to do a one-time removal of the field in question from our site when they do a removal (i.e. update the person table and remove that field), and aggressively warn that this serves only to accelerate the process of removal after removing one's data from the Face Book/directory. We really, really should not be putting ourselves in a position of needing to remove fields at request-time. I doubt there is any way to make that efficient.
| @app.route('/edit', methods=['GET']) | ||
| @requires_login | ||
| def edit(): | ||
| if g.person is None: |
There was a problem hiding this comment.
I think we check this already
| return render_template('edit.html') | ||
| return render_template( | ||
| 'edit.html', | ||
| socials_instagram=person_persistent.socials_instagram, |
There was a problem hiding this comment.
I think that you can do **person_persistent.__dict__ instead of enumerating all these
There was a problem hiding this comment.
Although it won't really matter if you remove the privacy_hide_* fields, which I strongly believe you should do
| elasticsearch.indices.delete(index=Person.__tablename__) | ||
| elasticsearch.indices.create(index=Person.__tablename__) | ||
|
|
||
| if elasticsearch is not None: |
There was a problem hiding this comment.
Great stuff thanks for fixing this
| const data = {}; | ||
| for(const field of FIELDS) { | ||
| const elem = document.getElementById(field); | ||
| if(elem.type === 'checkbox') data[field] = elem.checked; |
There was a problem hiding this comment.
Nit: put space in between if, for, etc. and the paren to match rest of the code
| </div> | ||
|
|
||
| <!-- | ||
| Snapchat feature vetoed by YCS Board |
There was a problem hiding this comment.
If so then let's remove
| ] | ||
|
|
||
| def scrub_hidden_data(person): | ||
| if hasattr(person, "privacy_hide_image") and getattr(person, "privacy_hide_image"): |
There was a problem hiding this comment.
As mentioned I disagree with this methodology
| setattr(person, "birth_day", None) | ||
| return person | ||
|
|
||
| INSTAGRAM_ALLOWED_CHARSET = set(string.ascii_letters + string.digits + '._') |
There was a problem hiding this comment.
I think this would fail the style test - can you run ./style_test.sh and fix any issues that pop up?
|
Superseded by #264 |
Description
Fixes #232
Fixes #206
Fixes #231
This PR adds the ability to hide information from the Yalies.io website, using a new profile editor page. Also, you can add your Instagram/Snapchat handle which will be displayed when someone searches you up.
TODO
This code reduces performance of people queries. Since the persistent data is stored in another table, for each person result, the secondary table has to be queried. There might be a way to optimize this.
You can't see the performance impact on the yalies website because pagination keeps it to a limited number of people per query, but if you use the API to query all people in the directory, it takes significantly longer than before.
Checklist