Skip to content
Commits on Feb 16, 2016
  1. @csilvers

    Fix the view-email to have the right url again.

    csilvers committed Feb 15, 2016
    In 3336cb6 I had this diff:
    ```
    -   http://weekly-snippets.appspot.com/weekly
    +   {{hostname}}
    ```
    That's not right!
    
    Auditors: oliver
    
    Test Plan:
    Fingers crossed
Commits on Jan 25, 2016
  1. @csilvers

    Add a bunch of TODO's to make operations more atomic.

    csilvers committed Jan 25, 2016
    http://weekly-snippets.appspot.com/weekly?week=01-18-2016 lists
    benkraft's snippets twice.  Maybe this is due to a non-atomic save.
    
    Auditors: michelle
    
    Test plan:
    None.  I basically just grepped for '.put()' in this repo.
Commits on Jan 20, 2016
  1. @csilvers

    Don't claim I wrote the snippet-server.

    csilvers committed Jan 19, 2016
    I probably didn't -- I don't remember that far back to be sure -- and
    http://blog.idonethis.com/google-snippets-internal-tool attributes it
    to schwim.  Whatever.  It's not important to the readme anyway.
    
    Auditors: michelle
Commits on Jan 19, 2016
  1. @csilvers

    Add an actual README, in preparation for opensourcing.

    csilvers committed Jan 19, 2016
    Summary:
    Add an "administration" section of the README.
    
    This made me realize there was another KA-specific setting still
    lurking in our codebase, in a yaml file, so I got rid of that too.
    
    While in the area, I also got rid of the obsolete --oauth2 appcfg
    flag.
    
    Test Plan:
    Will make this a pull request, and we can see how it renders!
    
    make deploy APP=weekly-snippet-hrd
    
    Reviewers: riley, michelle
    
    Reviewed By: riley, michelle
    
    Subscribers: mroth, benjaminpollack
    
    Differential Revision: https://phabricator.khanacademy.org/D24682
Commits on Jan 13, 2016
  1. @csilvers

    Move the hostname from being hard-coded to being a app setting.

    csilvers committed Jan 12, 2016
    Summary:
    This is the last piece (I know of) where we are hard-coding our
    dependency on our app and where it lives.
    
    Test Plan:
    make check
    
    Also ran
       make serve
    and visited
       localhost:8080
    as a logged-in admin user.  I made sure there were no existing
    datastore entities (using localhost:8000) and got sent to the
    app-settings screen.  It defaulted to 'http://localhost:8080'
    for the hostname setting, which is what it should be.
    
    Reviewers: riley, michelle
    
    Reviewed By: michelle
    
    Subscribers: mroth, benjaminpollack
    
    Differential Revision: https://phabricator.khanacademy.org/D24556
Commits on Jan 12, 2016
  1. @csilvers

    Fix /admin/manage_users to not need every snippet for every user.

    csilvers committed Jan 12, 2016
    Summary:
    The endpoint just needs the latest snippet for each user.  I added a
    new endpoint (and a new index), to allow that.
    
    I also changed the wording a bit of the 'weeks ago' text to be more
    english-like.
    
    Test Plan:
    make test
    
    Also ran 'make serve' and visited
       http://localhost:8080/admin/manage_users
    as a logged-in admin-user who had written several snippets.  It said:
    "1 week ago" under "Last snippet" meaning that we're still reading the
    snippet info ok.
    
    Reviewers: riley, michelle
    
    Reviewed By: michelle
    
    Subscribers: mroth, benjaminpollack
    
    Differential Revision: https://phabricator.khanacademy.org/D24541
  2. @csilvers

    Just use target, not toElement.

    csilvers committed Jan 12, 2016
    This is according to the wisdom of
      http://stackoverflow.com/questions/8600174/event-toelement-in-ie8-and-firefox
    
    Auditors: michelle
    
    Test plan:
    Ran 'make serve' and visited
       http://localhost:8080/admin/manage_users
    as a user logged in as an admin, and tried to hide a user, and hit
    reload and saw the button now said 'unhide' (so it worked!)
  3. @csilvers

    Add .pyc files to skip_files to avoid some warning messages from GAE.

    csilvers committed Jan 12, 2016
    This isn't necessary -- GAE will skip .pyc files anyway -- but it's
    prettier when doing the deploy.
    
    Auditors: michelle
    
    Test plan:
    make deploy
  4. @csilvers

    Use ajax to send hide/delete requests.

    csilvers committed Jan 12, 2016
    Summary:
    Added some basic error-checking, and made it work on firefox.
    
    I don't know the best way to turn an event into an element, but I
    found a way that works on two different browsers at least.  Hopefully
    that's enough...
    
    The error checking is //very// basic, fwiw.
    
    I also made the class names more specific, so we don't match other
    things accidentally.
    
    Test Plan:
    Ran 'make serve' and visited
       http://localhost:8080/admin/manage_users
    as a logged-in-as-admin user (which I had already created).
    
    I then clicked 'hide' for my one user and saw it change to 'hiding...'
    and then 'unhide', I reloaded the page and saw it say 'unhide',
    meaning the setting had stuck.  I then clicked on 'unhide' and saw it
    change back.
    
    I then clicked on 'delete' and saw it change to 'deleting...' and then
    'deleted', and reloaded the page and saw that that user was gone.
    
    Fixes #15
    
    (same)
    
    Reviewers: riley, michelle
    
    Reviewed By: michelle
    
    Subscribers: mroth, benjaminpollack
    
    Differential Revision: https://phabricator.khanacademy.org/D24481
Commits on Jan 11, 2016
  1. @csilvers

    Add the ability to hide/delete yourself.

    csilvers committed Jan 11, 2016
    Summary:
    The hardest part was trying to find the right shade of yellow!
    
    Fixes #10
    
    Use bootstrap colors (http://getbootstrap.com/css/#buttons)
    
    Suggested by michelle
    
    Test Plan:
    make check
    
    (none new)
    
    Reviewers: riley, michelle
    
    Reviewed By: michelle
    
    Subscribers: mroth, benjaminpollack
    
    Differential Revision: https://phabricator.khanacademy.org/D24463
Commits on Jan 8, 2016
  1. @csilvers

    Get rid of the (few) references to Khan Academy in the codebase.

    csilvers committed Jan 8, 2016
    I did not modify tests.  I also left the README alone; it will need a
    more major update later.
    
    Auditors: mroth
    
    Test Plan:
    None
Commits on Jan 6, 2016
  1. @allofthenorthwood

    Make app setting inputs full-width so you can see the full tokens

    allofthenorthwood committed Jan 6, 2016
    Test Plan:
    Logged in as admin and visited http://localhost:8080/admin/settings
    {F229550}
    
    Reviewers: csilvers
    
    Reviewed By: csilvers
    
    Differential Revision: https://phabricator.khanacademy.org/D24399
  2. @csilvers

    Use a big stick to make sure tests never send to chat.

    csilvers committed Jan 6, 2016
    I also fixed a test that got broken in a merge conflict somewhere.
    
    Auditors: mroth, benjaminpollack
    
    Test Plan:
    make check
    Then looked at the #khan-academy slack room and saw no messages
    posted. :-)
  3. @csilvers

    Get rid of the cfg-check in the Makefile: we've nixed the config files.

    csilvers committed Jan 6, 2016
    Auditors: mroth
    
    Test Plan:
    make deploy
  4. @csilvers

    Move hipchat/slack integration from config files to datastore values.

    csilvers committed Jan 6, 2016
    Summary:
    Now you can use /admin/settings to configure hipchat and slack
    integration.  This makes it much easier to administer them without
    needing to redeploy the app, and it also simplifies the app logic for
    when to send to hipchat/slack and when not to.
    
    Fix the name of the chat cronjob.
    
    It's no longer hipchat-specific.
    
    Minor refactors based on review.
    
    1) Move the slack-formatting out of command_usage() and into the
    caller, to simplify the other use of command_usage() in
    app-settings.html.
    
    2) Use stuff after the last @ as the domain more consistently
    
    3) Fix a few tests I accidentally broke after a last-minute function
    rename.
    
    Test Plan:
    make check
    
    (none)
    
    make check
    
    Reviewers: benjaminpollack, mroth
    
    Reviewed By: mroth
    
    Subscribers: riley, michelle
    
    Differential Revision: https://phabricator.khanacademy.org/D24375
  5. @csilvers

    Add an app-wide settings page, and use it to control access to the site.

    csilvers committed Jan 6, 2016
    Summary:
    Now new users can only join if they are from one of the domains listed
    on the app-settings page.  That page also controls various other
    defaults.
    
    Make a reasonable onboarding experience.
    
    Now, when you run the snippet-server for the first time, it redirects
    you to the app-setup page, then from there to the user-setup page.
    
    Add tests, and change code to not create a user until settings are committed.
    
    Before, as soon as you visited /settings for a user that user was
    created.  Now the user isn't created until those settings are saved.
    I think that matches expected behavior better.
    
    I also added a helper routine to get the application-wide settings,
    rather than doing a low-level get() every time I wanted them.
    
    Use an explicit key-name to simplify (and speed up) app-settings get/put's.
    
    Improve domain parsing to allow whitespace to separate domains as well.
    
    I also improved wants_to_watch parsing to match, even though that
    param isn't used anywhere yet.
    
    Test Plan:
    TODO
    
    Still TODO
    
    make test
    
    (same)
    
    make check
    
    Reviewers: michelle, riley, benjaminpollack
    
    Reviewed By: benjaminpollack
    
    Differential Revision: https://phabricator.khanacademy.org/D24346
Commits on Dec 22, 2015
  1. @allofthenorthwood

    Bump css version

    allofthenorthwood committed Dec 22, 2015
    Test plan:
    New css! Not much to test.
    
    Auditors: riley
  2. @allofthenorthwood

    Add a gravatar photo next to each snippet

    allofthenorthwood committed Dec 22, 2015
    Test Plan:
    {F224893}
    Also added instructions to your settings on how to change it:
    {F224895}
    
    Reviewers: riley
    
    Reviewed By: riley
    
    Differential Revision: https://phabricator.khanacademy.org/D24262
  3. @allofthenorthwood

    Style `hr`s

    allofthenorthwood committed Dec 22, 2015
    Summary: Because someone keeps using them in their snippets! :)
    
    Test Plan: {F224875}
    
    Reviewers: riley
    
    Reviewed By: riley
    
    Differential Revision: https://phabricator.khanacademy.org/D24260
  4. @rileyjshaw

    Bump CSS version

    rileyjshaw committed Dec 22, 2015
    (Made it v3 to match the current js version, but going forward they shouldn't need to match)
    
    Test Plan:
    
    - Check that new tags are styled
    
    Auditors: michelle
  5. @allofthenorthwood

    Show all snippet previews regardless of dirty state

    allofthenorthwood committed Dec 22, 2015
    Test Plan:
    Look at those previews!
    {F224859}
    
    Reviewers: riley
    
    Reviewed By: riley
    
    Differential Revision: https://phabricator.khanacademy.org/D24259
  6. @rileyjshaw

    Rewrite snippets.js

    rileyjshaw committed Dec 22, 2015
    Summary:
    As we've added more functionality to snippets.js, it's become a bit of a soup. After a non-obvious-but-should-have-been-obvious bug with event targets landed yesterday, it seemed time to restructure some of the copypastas in snippets.js. Hopefully this makes adding new features to the User Snippets page a bit less intimidating going forward :)
    
    Speaking of which, this adds a feature! As mentioned in D24254,
    
     > Known issues: toggling private / markdown state doesn't register a snippet as "dirty". This has never worked, as far as I can tell.
    
    Now it does! Checkbox state is recorded, so they now contribute to a snippet's "dirty" status. They also reset when "Clear Changes" is clicked.
    
    I still think that a React rewrite would be awesome, and if this file goes away //tomorrow// I'd be happy. But for now, hopefully it's a step in the right direction.
    
    Depends on D24254.
    
    Test Plan:
    - Clicked around and typed a bunch with the console open,
    - would appreciate a second pair of eyes on it if you've got a few minutes.
    
    Reviewers: michelle
    
    Reviewed By: michelle
    
    Subscribers: anju
    
    Differential Revision: https://phabricator.khanacademy.org/D24257
  7. @rileyjshaw

    Restrict user snippet actions to a single form

    rileyjshaw committed Dec 21, 2015
    Summary:
    D24239 caused a bug where event handlers from `snippets.js` affected //every// snippet on the page. This diff ensures that each action on the user snippets page only affects the related snippet.
    
    Specifically, this diff uses `.closest()` and `$parentForm.find()` more consistently throughout `snippets.js`.
    
    Test Plan:
    - Open http://localhost:8080/
    - Create a bunch of snippets by:
      - Inspecting an existing snippet.
      - Editing the hidden `input`'s `value` attr to a new date.
      - Saving.
    - Edit [single|multiple] snippets by changing their [private|markdown|textarea] value.
    - `make test`
    
    Known issues: toggling private / markdown state doesn't register a snippet as "dirty". This has never worked, as far as I can tell.
    
    Reviewers: michelle
    
    Reviewed By: michelle
    
    Subscribers: anju
    
    Differential Revision: https://phabricator.khanacademy.org/D24254
  8. @rileyjshaw

    Add "No Snippet" and "Private" tags

    rileyjshaw committed Dec 18, 2015
    Summary:
    This diff accomplishes two major goals by adding snippet tags (screenshot included). Before this diff,
    
    - There were oodles of "(no snippet this week)" entries which took up an unnecessary amount of vertical space, and,
    - Private snippets (#888) were pretty hard to read (issue #4).
    
    This diff also changes a bit of the snippet preview logic to display the new tags dynamically.
    
    A good follow-on project might be grouping the No Snippet-ers to take up even less space on the page.
    
    This is a bigger diff than I usually put out (and it's not documented all that well). If you actually //do// want me to split it up into smaller commits, the offer totally still stands :)
    
    Test Plan:
    - Add a new user
    - Add some snippets
    - Look at 'em on the User Snippets and Weekly Snippets pages
    - `make test`
    
    Reviewers: michelle
    
    Reviewed By: michelle
    
    Subscribers: anju
    
    Differential Revision: https://phabricator.khanacademy.org/D24239
Commits on Dec 14, 2015
  1. @rileyjshaw

    Update classnames in snippets_test to match D22001

    rileyjshaw committed Dec 14, 2015
    All tests should pass now! :tada:
    
    Test Plan:
    make test
    
    Auditors: michelle
  2. @rileyjshaw

    Update expected outcome of testCategoryUnsetButSnippetHasContent

    rileyjshaw committed Dec 14, 2015
    Since D22001, we've been hounding the user to change their category from (unknown) even if their snippet is filled out. This updates our tests to reflect that.
    
    Test Plan:
    make test
    
    Auditors: michelle
Commits on Dec 13, 2015
  1. @rileyjshaw

    Update category tests to match current markup

    rileyjshaw committed Dec 13, 2015
    Since 3ba4c30, `WARNING:` has been wrapped in <strong> tags. This updates our tests to match the new markup.
    
    Test Plan:
    make test
    
    Auditors: michelle
  2. @rileyjshaw

    Update string matching in checkbox tests

    rileyjshaw committed Dec 13, 2015
    The `assertInputIsChecked` and `assertInputIsNotChecked` methods were previously checking for specific whitespace. These tests were failing because of an indentation change.
    
    This patch allows flexible whitespace within the test strings.
    
    Test Plan:
    make test
    
    Auditors: michelle
  3. @rileyjshaw

    Add .unique-snippet class to user- and weekly- snippets for testing

    rileyjshaw committed Dec 13, 2015
    `assertNumSnippets` and `_ith_snippet` tests run on both user- and weekly- snippets, but were previously only checking a class that existed on user-snippets. This fixes that.
    
    Test Plan:
    make test
    
    Auditors: michelle
  4. @rileyjshaw

    Update tests to match f8293a2

    rileyjshaw committed Dec 13, 2015
    Test Plan:
    
    ```
    git checkout f8293a2
    <apply changes>
    make test
    ```
    
    Auditors: michelle
  5. @rileyjshaw

    Fix regression introduced in f1adb1a

    rileyjshaw committed Dec 13, 2015
    Test Plan:
    
    ```
    git checkout f1adb1a
    arc patch <this diff>
    make test
    ```
    
    Tests should now pass :)
    
    Auditors: michelle
Commits on Dec 10, 2015
  1. @rileyjshaw

    Remove extra spaces from user settings textarea

    rileyjshaw committed Dec 9, 2015
    Summary:
    `<textarea>`s preserve whitespace, so the indentation of *"Email addresses of people to view snippets of"* was rendering to the page.
    
    A huge change, really.
    
    Test Plan:
    - Navigate to /settings
    - Ensure excess padding no longer exists
    
    Reviewers: michelle
    
    Reviewed By: michelle
    
    Differential Revision: https://phabricator.khanacademy.org/D23919
Commits on Dec 8, 2015
  1. @allofthenorthwood

    Add JS version to url for cache-busting

    allofthenorthwood committed Dec 8, 2015
    Summary:
    
    Test Plan: This will cause everyone to re-download the js with the new changes without having to force refresh.
    
    Auditors: csilvers
  2. @allofthenorthwood

    Add CSS version for cache-busting

    allofthenorthwood committed Dec 8, 2015
    We could come up with something more sophisticated at some point, but this should at least get everyone on the same page for now.
    
    Test plan:
    This will cause everyone to re-download the css with the new changes without having to force refresh.
    
    Auditors: csilvers
  3. @allofthenorthwood

    Make sure images in snippets don't exceed the width of the page

    allofthenorthwood committed Dec 7, 2015
    Large images make the page scroll horizontally without this fix.
    
    Test Plan:
    Add a large image in your markdown snippet `![alt text](http://path.to/image)` and see that it doesn't exceed the width of the page.
    
    Auditors: csilvers
Something went wrong with that request. Please try again.