Persona research screener banner #8192
Conversation
jimchamp
force-pushed
the
8180/feature/screener-banner
branch
3 times, most recently
from
f650219 to
8076cc9
Compare
|
|
Possibly copy change?
|
| sorted_edu_domains = utils.get_edu_domains() | ||
|
|
||
| i = bisect_left(sorted_edu_domains, domain) | ||
| if i != len(sorted_edu_domains) and sorted_edu_domains[i] == domain: | ||
| return True |
There was a problem hiding this comment.
We may be able to use a set instead of a list and get constant time lookup
There was a problem hiding this comment.
Not able to cache sets currently, so have used a dict instead of a set.
| @@ -1314,6 +1314,16 @@ def get_location_and_publisher(loc_pub: str) -> tuple[list[str], list[str]]: | |||
| return ([], [loc_pub.strip(STRIP_CHARS)]) | |||
|
|
|||
|
|
|||
| @functools.cache | |||
| def get_edu_domains(): | |||
| f = open('static/edu-domains.txt') | |||
There was a problem hiding this comment.
I think somewhere in scripts we may have an e.g. of abspath being used (re: whether this relpath references the file as we want)
There was a problem hiding this comment.
We may want to...
- 1. check the relative edu file path (to see if it's working)
- 2. use a
setinstead of list for edu domain lookup (is email.split('@')[-1] in edu set?) -- add archive.org as domain (for testing)- Can we link to a gist that can be used to e.g. update this list of domains in the future if the corresponding 3rd party github repo changes?
- 3. (optional) move functools.cache to memcache w/ 1 mo (since webheads have limited RAM)
- 4. only set cookie if admin (for now)
- 5. add year=2023 to month=8 check (so we don't show banner in 2024)
- 6. (optional) move banner dismiss check to cookies rather than account prefs?
| @@ -1314,6 +1314,16 @@ def get_location_and_publisher(loc_pub: str) -> tuple[list[str], list[str]]: | |||
| return ([], [loc_pub.strip(STRIP_CHARS)]) | |||
|
|
|||
|
|
|||
| @functools.cache | |||
| def get_edu_domains(): | |||
There was a problem hiding this comment.
| def get_edu_domains(): | |
| def get_edu_domains(): | |
| """ | |
| This list created using this gist: https://gist.github.com/jimchamp/f67ef14c3bcf11593f393ee5288f6476 on https://raw.githubusercontent.com/Hipo/university-domains-list/master/world_universities_and_domains.json. | |
| See: https://github.com/internetarchive/openlibrary/issues/8180 | |
| """ |
mekarpeles
added
the
Priority: 1
|
|
||
| # Don't overwrite the cookie, which will contain banner display preferences | ||
| if not web.cookies().get('se', False): | ||
| self.set_screener_cookie(ol_account) | ||
|
|
There was a problem hiding this comment.
Important Note
We are determining if a patron is eligible for the survey whenever they log in. The problem with this approach is that people who chose the "Remember me" option while logging in will likely not see the banner.
Not sure how to address this issue. Do we have a means of logging everybody out at once? Doing so before the campaign would ensure that everybody who is eligible and visits will see the banner, but would also be an annoyance for our patrons.
Maybe we set the screener cookie on the My Books page instead of at login? This would avoid logging everybody out, but we may still miss eligible folks who seldomly visit their My Books page.
There was a problem hiding this comment.
This is a good point. If we do a deploy that might log people out? For now, maybe it's okay if we ignore this case (let's see how many patrons reply). It's a really good point and there are ways to log everyone out.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
- Add `archive.org` to educational domains (for testing purposes) - Store edu domains in a `dict` for constant-time look up - Cache edu domains using memcached - Only display banners during August 2023 - Only display banners for `admin`s (for testing purposes) - Update banner copy
jimchamp
force-pushed
the
8180/feature/screener-banner
branch
from
8aa7a4c to
ffcaf39
Compare
| $# Start : Persona research banner | ||
| $if user: | ||
| $if user.is_screener_eligible(): | ||
| $ banner_key = 'pr2308' |
There was a problem hiding this comment.
Read as: Persona Research 2023-08
jimchamp
changed the title
|
|
||
| See: https://github.com/internetarchive/openlibrary/issues/8180 | ||
| """ | ||
| p = Path(Path.cwd(), 'static', 'edu-domains.txt') |
There was a problem hiding this comment.
If we're unsure about setting the path this way, we could do something like this:
openlibrary/openlibrary/plugins/openlibrary/code.py
Lines 224 to 226 in f3c7db2
| # It is August 2023: | ||
| if (now := datetime.now()) and now.month != 8 and now.year != 2023: | ||
| return False |
There was a problem hiding this comment.
We may need to update these dates, as time is running short.
|
|
||
| See: https://github.com/internetarchive/openlibrary/issues/8180 | ||
| """ | ||
| p = Path(Path.cwd(), 'static', 'edu-domains.txt') |
There was a problem hiding this comment.
(Side note; the / operator can be used here!)
| p = Path(Path.cwd(), 'static', 'edu-domains.txt') | |
| p = Path.cwd() / 'static' / 'edu-domains.txt' |
jimchamp
added
the
On testing.openlibrary.org
|
Noting that the edu domain textfile is not being added when the PR deployment script is used. |
jimchamp
removed
the
On testing.openlibrary.org
Closes #8180
On login, checks a patron's eligibility to be shown a CTA to respond to a persona research survey. If eligible, a new cookie will be set. If the cookie is set, a dismissible banner will be shown on the patron's "My Books" page.
To be eligible, each of the following must be true:
Notes about the data set:
Date accessed: 10 August 2023
Data set size: ~2.7 MB
Flattened data set size: ~125.9 kB
Number of institutions: 9936
Number of domains: 10180
Remaining tasks
hrefof banner linkTechnical
Script used to flatted the edu domains: https://gist.github.com/jimchamp/f67ef14c3bcf11593f393ee5288f6476
Testing
Screenshot
Stakeholders