Giter Site home page Giter Site logo

commits.to's Introduction

Commits.to — a.k.a. The I-Will System

CircleCI codecov Maintainability Test Coverage

Start with the Functional Spec which also gives the backstory for this project.

Development Setup

Follow the steps below. Please make a pull request if any of this isn't super straightforward or you need to do additional steps to get up and running!

Set Up The Database

Install Postgres

Environment Command
macOS brew install postgresql
Linux sudo apt install postgresql

Start Postgres

macOS

Start it with running either brew services start postgresql to have it as a background service that will restart if you reboot or pg_ctl -D /usr/local/var/postgres start to start it just once.

Ubuntu

Start it with the command sudo service postgresql start to have it run as a background service that will restart if you reboot or sudo -u postgres /usr/lib/postgresql/10/bin/pg_ctl -D /var/lib/postgresql/10/main -l logfile start which runs the script as the automatically created PostgreSQL user account to start it just once.

Confirm that postgresql is running on localhost:5432. If you run pg_isready you should see /tmp:5432 - accepting connections.

Create Database

Run the following to create a user and a database. If prompted for a password, use the password iwill.

macOS
createuser -P iwill
createdb -O iwill commitsto
Ubuntu
sudo -u postgres createuser -P iwill
sudo -u postgres createdb -O iwill commitsto

Install Dependencies

2.1 Install Node 8.x LTS with the binary or installer or by using a package manager.

Create Environment File

Create a .env file in the root of the project directory with the following contents, replacing <yourname> with your name:

APP_PORT=5000

DATABASE_URL=postgres://iwill:iwill@localhost:5432/commitsto

Run The Application

If you are running the app for the first time, or have recently pulled changes, you should run npm install

Start each process in a separate terminal pane

  • Build server in watch mode: npm run dev:build:server
  • Build client in watch mode: npm run dev:build:client
  • Start server: npm run dev:server

Try The App

Just navigate to localhost:5000/

Troubleshooting

If you have problems connecting to the application via your browser, check that you have made the necessary changes to the /etc/hosts file for the subdomain you are using.
You may also need to flush the DNS cache to ensure those changes are recognized.

Ubuntu

On Ubuntu, you can use the name service cache daemon (nscd) to flush the DNS cache.

sudo apt-get install nscd
sudo service nscd restart
Mac

On MacOS, the command to flush the DNS cache will depend on your exact OS level. See How To Clear Your DNS Cache for detailed recommendations by OS version.

Testing

Run Mocha tests with npm test.

Writing tests

We're using the Chai assertion library and Sinon for spying/stubbing. You can run tests in watch mode to get results nearly instantly on save with npm run test:watch

Structuring tests

The structure and naming inside the test/ folder should mirror the root structure and file names. Writing code with well-contained classes or functions will be the most straightforward to unit test.

Deployments

Automatic Heroku Deployment

Environment Branch Domain
Staging master http://commitstew.com
Production production http://commits.to

Issue Tracking

Issue tracking labels follow the Beeminder Label Ontology. The following abbreviations, acronyms, and amusing shorthands are employed in service of bug zapping and feature enhancing:

Open Labels

Label Meaning
BUG Opposite of feature
RFE Request For Enhancement, aka feature request
UVI User-Visible Improvement
STY Style/polish/CSS, or think of it as in pigsty or an eyesore
MEN Mendoza = need to resolve before accepting more beta users
PEA Easy-peasy
SKY Pie in the sky (would be awesome but not necessarily worth the effort)
ABC Non-technical, like prose or webcopy tweaks
ADO Consensus needed on what to Actually Do (or "much ado about ∅"), AKA question

Closed Issues

Label Meaning
aok Feature, by design
cnr Could not reproduce
dup Duplicate
nix Won't fix or invalid
zap fixed
zzz postponed

commits.to's People

Contributors

alys avatar byorgey avatar chrisbutler avatar clivemeister avatar dependabot[bot] avatar dreeves avatar kschachter avatar lithp avatar mikelake avatar pjjh avatar yebyen avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

commits.to's Issues

IPAPI rate limiting

I believe this is periodically breaking the app -- all promise URLs redirect to the user's gallery.

ipapi response: {
  'reason': 'RateLimited', 
  'wait': 1399.0, 
  'message': 'SignUp / Upgrade your plan @ https://ipapi.co/', 
  'error': True }

Warn if navigating away without saving

I forget what this design pattern is called but I think it's pretty standard and pretty important. If you change something in a form and then try to leave without clicking Submit/Save then it should yell at you.

HT kim who got bitten by this

Tini gets spuriously rounded when saving

Steps To Reproduce

  1. At some odd time like 7:57pm, create a promise like alice.commitsto.review/test-by-8:03pm
  2. Note that tini and tdue appear correct to the minute
  3. Click SAVE

Expect

We expect tini and tdue to stay correct

WTF

They both jump forward to the next even half hour.

I have use cases where the exact time of both tdue and tini matters a lot so this feels like a high severity bug despite the errors involved being at most 29 minutes.

Hover effects on promise cards are confusing

I clicked next to the “mark complete” text and the hover effects made it seem like I was successfully clicking it and that it had silently failed. In reality I just had to click the actual words “mark complete” but it seemed like a nasty silent-failure bug until I realized that.

Reliable reliabilities

This is just a caching issue but it keeps freaking people out and I think counts as a fairly serious bug since seeing people's up-to-the-second reliability is a big incentive to click on promise URLs.

It's hard to reproduce on demand but it happens all the time and I think the easiest place to see it is on the main commits.to page. People's reliabilities there will often be much lower than what you see when you click through to the person's gallery.

Reject most special characters

We're debating what characters should be allowed in URLs but I think that rejecting any route that matches the following should be non-controversial, though also rare enough that it may not matter much, at least while we just have a small number of beta users:

/.*[\!\%\$\^\*\(\)\[\]\=\+\{\}\\\|\;\'\"\`\~\&].*$/

I.e., reject exclamation marks, percents, dollar signs, carets, asterisks, equal signs, pluses, pipes, semicolons, tildes, ampersands, backslashes, parens, brackets, braces, single quotes, double quotes, and backticks.

(Of course better would be to have a regex specifying exactly what routes are accepted and reject everything else.)

After we're rejecting the noncontroversial characters we can make a new Issue to tackle migrations (https://github.com/beeminder/iwill/wiki#legacy-migration-for-urls-with-slashes).

remove '/edit' scrubbing from urtext parsing

This should solve some remaining problems with rogue promises from crawlers.

Once there's no references to /edit routes in any links, crawlers should stop trying those. So I think we can just throw away all the code for the old /edit stuff and we'll be all set. Possibly a special case to reject anything ending in /edit in the meantime while the crawlers are still trying those. Or of course rejecting anything with a dot would also solve the problem.

Promise page improvements

When you go to alice.promises.to/xyz it says "alice has promised to 'xyz' by [date]" with a nice countdown timer to the deadline to give a feeling of a ticking clock.

Also it should show Alice's reliability statistics. Maybe it shows Alice's full list of promises.

Allow changing due date when confirming promise

Let's not blindly trust our parsing of the date from the URL, or even make it important to include a date.

Proposal: give the user (need not be a logged-in user) the chance to edit the due date when confirming the promise.

Did bob just promise to 'xyz' by [date-picker]?

The datepicker uses the /by/fri or whatever from the URL as the default but if it parsed it wrong you have a chance to change it. If no date was included, default to now + 7 days.

Again, anyone hitting the URL can confirm the promise and pick the due date and it should be worded so as to encourage the promisee (whoever Bob is making the commitment to) to do so, though of course Bob himself can click it first and confirm it if he wants.

All subsequent visits to the URL just say "bob has promised to 'xyz' by [date]" and maybe also a nice countdown timer to the deadline to give a feeling of a ticking clock, but that's a separate issue, so to speak.

remove unique urtext constraint

Steps to Reproduce the Bug

  1. Make up a unique slug like "test2142"
  2. Click alice.commitsto.review/test2142
  3. Click carol.commitsto.review/test2142

What I Expected

Both promises are created and they have nothing to do with each other.

What Happens

The 2nd one crashes the app.

Some SQL query is accidentally considering only the urtext and not the :user/:urtext somehow.

Case sensitivity of urtexts

Let's conform to the official protocol for URLs: Domains are case-insensitive, paths are case-sensitive. So Alice.commits.to/aBc gets rewritten to alice.commits.to/aBc.

The urtext should be what it says -- the original text, exactly as the user typed it.

fix handling URLs with leading dashes

Should alice.commits.to/foo-bar and alice.commits.to/foo_bar be canonicalized to the same thing?

I'm inclined to advocate for the other extreme of canonicalizing nothing. alice.commits.to/abc and alice.commits.to/ABC and alice.commits.to/Abc should all be entirely distinct. Same for alice.commits.to/_foo and alice.commits.to/foo.

By the worse-is-better design philosophy, these questions should be heavily influenced by what yields the cleanest, simplest implementation. I'll let @chrisbutler confirm if dropping all canonicalizing would simplify the code.

I believe case-insensitivity is the default with Express but there's a setting to turn it off. One of these:

app.set('case sensitive routing', true)
router = express.Router({ caseSensitive: true })

I've also heard someone say you need to use regex-based routing instead of string-based routing to make case-sensitivity work. I doubt that's true but there may be other reasons to prefer regex-based routing anyway.

Get timezone from browser

For modern browsers there’s something built in to Javascript that should work:

Intl.DateTimeFormat().resolvedOptions().timeZone

That correctly returns America/Los_Angeles for me in Chrome and looks pretty safe to use in general: https://caniuse.com/#search=Intl

I gather there's a concern from @chrisbutler and @pjjh that geoIP/IPAPI may better handle the use case of someone traveling. I believe, contrariwise, that the above javascript is always the right thing to use. If you haven’t changed the timezone for your computer then we should treat that as intentional and not try to be clever and figure out where you really are by your IP address.

Reject unkosher URLs

Give the user an error if any of the following are true:

A. slug doesn't match /^[\w\-]+$/
B. there's a querystring
C. there are any slashes except for the /by/ part

Test cases:

  1. bob.promises.to/do_a_thing/by/fri_5pm → GOOD
  2. bob.promises.to/do-a-thing/by/tomorrow → GOOD
  3. bob.promises.to/foo%bar → BAD
  4. bob.promises.to/foo^bar → BAD
  5. bob.promises.to/do_item_#2/by/9am → BAD

Note that the last one, handling a # character in the URL, is going to be hard: https://stackoverflow.com/questions/18796421/capture-anchor-link

Delete link is too dangerous right now

With the edit form wide open on the promise page (which is fine, maybe even preferable based on the frequency I need to edit things currently) I fear the Delete link is too scary to just have sitting there. And now that we have proper voiding, maybe it's not necessary. When we have proper user auth (#21) maybe the logged in user should have the choice to totally delete something, but not in the meantime.

While we have only a handful of active users I propose the Delete link be like "What happened that you want to delete this? Tell us about the use case and we’ll manually take care of it."

Labeling this MENdoza since we can't accept more beta users till it's addressed in some way.

Email each new promise to the user

I think this is important for making users feel assured that they always have a record (that they're in full control of) of everything they've committed to. Especially before we have user logins (#21) and while anyone can totally delete anything from the database (#143).

Personally I wouldn't find it too spammy and would really like to see an email for each promise. I could also then snooze that email as another way to remind myself to fulfill the commitment.

Note that we need an email field in the user table for this.

Store IP address of promise's creator in the database

This is sort of related to #169 in that we currently use the IP address to infer timezone.

If we wanted to conform to the naming convention we could call it urip but that's a bit cryptic and there are plenty of other things (browser, operating system) that may be worth saving too and we probably don't want to conform to the naming convention for all of those.

Commitments specifying "by day X" should default to the end of that day

I made this one:

http://nick.commits.to/give_feedback_on_commits_to/by/sunday

... expecting to do it on Sunday. It put it at 3am, great, that's the end of a day; could be 4am, could be midnight, whatever–those are fine defaults. But it interpreted it as 3am Sunday morning instead of 3am Monday morning.

screenshot 2018-01-28 14 54 24

Am I crazy or does saying you'll do something by Sunday mean you might do it on Sunday, not necessarily before Sunday?

(I just manually edited this one to be due 11:59pm.)

Don't redirect from gallery after completing a promise

If I complete a promise from my overview bee.commits.to page, it redirects me to the individual promise page after it marks it complete. It would make more sense and generally be preferable to stay on the same page I'm currently on after that action.

Laissez faire duedate parsing

Proposal for the duedate parser -- a function that takes the urtext (or technically the path) as a string and returns the timestamp of the duedate:

  1. replace dashes and underscores and slashes in urtext with spaces
  2. do some other handy substitutions like by dec5 -> by dec 5 and by (.*)tmw(.*) -> by $1tomorrow$2
  3. run the thus-transformed urtext through Sherlock and return the timestamp

Step 2 is optional and just lets us augment Sherlock's parsing in various ways we may find convenient, like maybe you want to say ..commits.to/foo/by/tmw or ..commits.to/foo/by/dec5.

(Tagging as Mendoza because we need to address in some way the fact that a lot of promises currently don't adhere to the /by/ syntax before inviting new users. Need to either be robust or enforce a rigid syntax.)

Part of promise name on card is dropped

UPDATE: This is the original gissue with #191 having additional technical discussion but we'll call that one the dup. Here's the collection of examples we noticed that exhibited the bug:

  1. ../set_alarms_for_restricted_eating_by_9pm -> "set alarms
  2. ../testing_this_thing_for_another_thing_by_9pm -> "testing this thing"
  3. ../send-an-email-on-DLC-by-midnight -> "Send an email"
  4. ../email_habitica_users_re_challenge_todos_within_a_week_of_code_ready -> "email habitica users re challenge todos within of code ready"
  5. ../create_an_evening_journal_beeminder_goal_by_19_aug_2018 -> "create an journal beeminder goal"

Original gissue description from Clarissa:

The url http://clarissa.commits.to/set_alarms_for_restricted_eating_by_9pm# ends up corresponding to a title on the card of "set alarms".

I tried another url, http://clarissa.commits.to/testing_this_thing_for_another_thing_by_9pm, and the same thing happened. The issue seems to be the word "for".

Incorrect tini

Steps to reproduce

  1. At, say, 12:43pm click alice.commitsto.review/test_by_1pm

Expected

The "commitment made" aka tini field should be 12:43pm

WTF

The tini field is some amount of time in the past. Sometimes an hour, sometimes like a half an hour. But it should always match the createdAt timestamp exactly, I think.

Commentary

This can occasionally be a big deal in terms of data integrity or looking up the exact time a promise was made in order to remember why you made it or where you would've been, etc.

Alex Strick crashes the server by creating promises

Happens consistently for user alex, who's in Amman, Jordan, maybe on a VPN. My only idea so far is that we're getting null or something from the IPAPI lookup and not handling that well.

I captured the logs around when Alex reproduced the crash:

2018-04-02T16:04:35.830042+00:00 app[web.1]: Executing (default): SELECT "id", "bmid", "urtext", "slug", "what", "firm", "void", "cred", "tini", "tdue", "tfin", "xfin", "clix", "note", "createdAt", "updatedAt", "userId" FROM "promises" AS "promises" WHERE "promises"."id" = 'alex/test_commits_system_tomorrow';
2018-04-02T16:04:36.120767+00:00 heroku[router]: at=info method=GET path="/test_commits_system_tomorrow" host=alex.commits.to request_id=9e3b6588-9735-49e0-9485-2ba2265bde8c fwd="46.29.218.185" dyno=web.1 connect=4ms service=308ms status=200 bytes=2475 protocol=http
2018-04-02T16:04:36.103581+00:00 app[web.1]: �[36m[2018-04-02 16:04:36.103] [DEBUG] �[39m�[36mipapi response: None�[39m
2018-04-02T16:04:36.103975+00:00 app[web.1]: �[36m[2018-04-02 16:04:36.103] [DEBUG] �[39m�[36mparsePromise start alex /test_commits_system_tomorrow�[39m
2018-04-02T16:04:36.104298+00:00 app[web.1]: �[36m[2018-04-02 16:04:36.104] [DEBUG] �[39m�[36mparsePromise replace test_commits_system_tomorrow /^[-\/._]+|[-\/._]+$/g�[39m
2018-04-02T16:04:36.104505+00:00 app[web.1]: �[36m[2018-04-02 16:04:36.104] [DEBUG] �[39m�[36mparsedText test_commits_system_tomorrow test commits system tomorrow�[39m
2018-04-02T16:04:36.105020+00:00 app[web.1]: �[31m[2018-04-02 16:04:36.104] [ERROR] �[39m�[31mpromise parsing error failed to parse timezone from IP�[39m
2018-04-02T16:04:36.117308+00:00 app[web.1]: �[31m[2018-04-02 16:04:36.117] [ERROR] �[39m�[31mpromise finding error�[39m
2018-04-02T16:04:36.128512+00:00 app[web.1]: _http_outgoing.js:491
2018-04-02T16:04:36.128515+00:00 app[web.1]:     throw new Error('Can\'t set headers after they are sent.');
2018-04-02T16:04:36.128517+00:00 app[web.1]:     ^
2018-04-02T16:04:36.128519+00:00 app[web.1]: 
2018-04-02T16:04:36.128520+00:00 app[web.1]: Error: Can't set headers after they are sent.
2018-04-02T16:04:36.128524+00:00 app[web.1]:     at validateHeader (_http_outgoing.js:491:11)
2018-04-02T16:04:36.128526+00:00 app[web.1]:     at ServerResponse.setHeader (_http_outgoing.js:498:3)
2018-04-02T16:04:36.128527+00:00 app[web.1]:     at ServerResponse.header (/app/node_modules/express/lib/response.js:767:10)
2018-04-02T16:04:36.128528+00:00 app[web.1]:     at ServerResponse.send (/app/node_modules/express/lib/response.js:170:12)
2018-04-02T16:04:36.128530+00:00 app[web.1]:     at done (/app/node_modules/express/lib/response.js:1004:10)
2018-04-02T16:04:36.128531+00:00 app[web.1]:     at Immediate._onImmediate (/app/node_modules/express-handlebars/lib/utils.js:26:13)
2018-04-02T16:04:36.128533+00:00 app[web.1]:     at runCallback (timers.js:794:20)
2018-04-02T16:04:36.128534+00:00 app[web.1]:     at tryOnImmediate (timers.js:752:5)
2018-04-02T16:04:36.128535+00:00 app[web.1]:     at processImmediate [as _immediateCallback] (timers.js:729:5)
2018-04-02T16:04:36.138925+00:00 app[web.1]: npm ERR! code ELIFECYCLE
2018-04-02T16:04:36.139207+00:00 app[web.1]: npm ERR! errno 1
2018-04-02T16:04:36.140368+00:00 app[web.1]: npm ERR! [email protected] start: `node babel.js`
2018-04-02T16:04:36.140474+00:00 app[web.1]: npm ERR! Exit status 1
2018-04-02T16:04:36.140646+00:00 app[web.1]: npm ERR! 
2018-04-02T16:04:36.140759+00:00 app[web.1]: npm ERR! Failed at the [email protected] start script.
2018-04-02T16:04:36.140861+00:00 app[web.1]: npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
2018-04-02T16:04:36.147115+00:00 app[web.1]: 
2018-04-02T16:04:36.147258+00:00 app[web.1]: npm ERR! A complete log of this run can be found in:
2018-04-02T16:04:36.147342+00:00 app[web.1]: npm ERR!     /app/.npm/_logs/2018-04-02T16_04_36_142Z-debug.log
2018-04-02T16:04:36.202750+00:00 heroku[web.1]: Process exited with status 1
2018-04-02T16:04:36.215690+00:00 heroku[web.1]: State changed from up to crashed
2018-04-02T16:04:38.267656+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/picker.css" host=alex.commits.to request_id=ee34fa04-efcf-4be7-8101-305e52e85ad7 fwd="46.29.218.190" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:04:38.259201+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/main.css" host=alex.commits.to request_id=7c09e130-4c37-4aa6-bf34-65cb820808a3 fwd="46.29.218.185" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:04:38.310753+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/js/picker.js" host=alex.commits.to request_id=39304208-e4c9-4051-9411-8cd09c33bcfb fwd="46.29.218.188" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:04:38.307937+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/js/promise.js" host=alex.commits.to request_id=a3f00c05-b653-4e57-b249-63bc19e6b48a fwd="46.29.218.190" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:04:38.308004+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/js/card.js" host=alex.commits.to request_id=cc04126c-a025-4aa5-b5d4-cd0683be2e85 fwd="46.29.218.185" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:04:40.800483+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/main.css" host=alex.commits.to request_id=6f61d72a-b17b-4938-a31a-396c2933238a fwd="46.29.218.189" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:04:42.791679+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/js/card.js" host=alex.commits.to request_id=442a6e0b-4f8f-4a60-a82c-05b174fe759e fwd="46.29.218.188" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:04:43.029490+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/js/promise.js" host=alex.commits.to request_id=48382a1d-6c50-4ef7-8cb3-2b2c64c6990e fwd="46.29.218.189" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:04:43.609929+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/js/picker.js" host=alex.commits.to request_id=7cbfb36a-3787-4740-8e06-0d5f7ccb6872 fwd="46.29.218.190" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:05:18.588153+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/test_commits_system_tomorrow" host=alex.commits.to request_id=e1774e8e-c4d4-41b9-a80a-02031fa6cf8b fwd="46.29.218.187" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:05:23.367502+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/favicon.ico" host=alex.commits.to request_id=f0126f11-7de3-4b19-a97d-c52d87685b6d fwd="46.29.218.188" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:06:07.472411+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/" host=alex.commits.to request_id=7c6f966d-05bc-44b6-8f56-40a5ff1b2d39 fwd="46.29.218.185" dyno= connect= service= status=503 bytes= protocol=http
2018-04-02T16:06:09.943644+00:00 heroku[router]: at=error code=H10 desc="App crashed" method=GET path="/favicon.ico" host=alex.commits.to request_id=ec1b5576-8cec-4f7b-9b18-624c49258e70 fwd="46.29.218.186" dyno= connect= service= status=503 bytes= protocol=http

Promise creation confirmation

Consider a promise, bob.commits.to/xyz/by/fri

Right now we're immediately creating a promise in the database as soon as the GET request happens on that URL. Instead do this:

Completely ignoring whether the user is logged in, show a huge

Did bob just promise to 'xyz' by [date]?

with a huge button that says "yes".

[EDIT: I moved the rest of this to separate Issues]

Route problem with multiple slashes

Bug report

Steps to reproduce

Surf to bob.commits.to/test/by/2017/10/30

What happens

404: Cannot GET /bob.commits.to/test/by/2017/10/30

Expected

{ "urtx": "bob.commits.to/test/by/2017/10/30",
  "user": "bob",
  "what": "test",
  "tdue": [whatever "2017/10/30" parses to],
}

Magically reappearing commitments

Something is happening that I don't understand. Ie, I have a faulty mental model of how our node/express/postgres stack fits together.

There are at least 7 URLs that keep magically reappearing under dreev.commits.to:

  • wp-includes/wlwmanifest.xml
  • site/wp-includes/wlwmanifest.xml
  • cms/wp-includes/wlwmanifest.xml
  • blog/wp-includes/wlwmanifest.xml
  • wp/wp-includes/wlwmanifest.xml
  • wordpress/wp-includes/wlwmanifest.xml
  • hirevmcyvpgypnk.html
  • robots.txt

I've created entries in the routes file to make them 404 (except robots.txt, which serves the actual contents of that file in /public). I also have some routes to do some redirects. After creating those routes and redeploying the app, I've deleted the above commitments from the database.

And then somehow they reappeared again!

How can that happen? I've repeated the above multiple times and feel I've ruled out PEBKAC but if anyone doesn't believe me I'll manually delete all the rogue stuff one more time and eat my hat if it doesn't reappear again. Which, again, from my understanding of how the routes work, should be impossible.

I'm hoping @chrisbutler can help generate hypotheses, otherwise my next step would be to store more information -- like originating IP and useragent -- with each promise, since we want to capture such things anyway. Either that or figure out how to look at Heroku's access logs. My initial attempts suggest maybe we don't store more than a few minutes of them.

Crash: foo//bar

Steps to Reproduce

  1. Click this in your dev environment: http://alice.commits-to.js:8080/foo//bar

What I Expected

Slashes being an allowed character currently, I'd expect it to create a promise. Complaining that two slashes in a row is not kosher would also be fine.

What Happens

TypeError: Cannot read property 'dataValues' of undefined
    at _express2.default.get (/Users/dreeves/prj/iwill/app/router.js:133:41)
    at Layer.handle [as handle_request] (/Users/dreeves/prj/iwill/node_modules/express/lib/router/layer.js:95:5)
    at next (/Users/dreeves/prj/iwill/node_modules/express/lib/router/route.js:137:13)
    at Route.dispatch (/Users/dreeves/prj/iwill/node_modules/express/lib/router/route.js:112:3)
    at Layer.handle [as handle_request] (/Users/dreeves/prj/iwill/node_modules/express/lib/router/layer.js:95:5)
    at /Users/dreeves/prj/iwill/node_modules/express/lib/router/index.js:281:22
    at param (/Users/dreeves/prj/iwill/node_modules/express/lib/router/index.js:354:14)
    at param (/Users/dreeves/prj/iwill/node_modules/express/lib/router/index.js:365:14)
    at param (/Users/dreeves/prj/iwill/node_modules/express/lib/router/index.js:365:14)
    at paramCallback (/Users/dreeves/prj/iwill/node_modules/express/lib/router/index.js:401:21)
    at _user.Users.findOne.then.user (/Users/dreeves/prj/iwill/app/router.js:25:14)
    at tryCatcher (/Users/dreeves/prj/iwill/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/Users/dreeves/prj/iwill/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/Users/dreeves/prj/iwill/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/Users/dreeves/prj/iwill/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/Users/dreeves/prj/iwill/node_modules/bluebird/js/release/promise.js:693:18)
    at Async._drainQueue (/Users/dreeves/prj/iwill/node_modules/bluebird/js/release/async.js:133:16)
    at Async._drainQueues (/Users/dreeves/prj/iwill/node_modules/bluebird/js/release/async.js:143:10)
    at Immediate.Async.drainQueues [as _onImmediate] (/Users/dreeves/prj/iwill/node_modules/bluebird/js/release/async.js:17:14)
    at runCallback (timers.js:756:18)
    at tryOnImmediate (timers.js:717:5)
    at processImmediate [as _immediateCallback] (timers.js:697:5)

Different background on initial creation

If you accidentally type a promise URL that already exists, that needs to be super obvious.

I propose we use a different background color on the page when clix=1.

(We could also make the "this promise has been clicked 1 time" more obvious or special-case it to "this promise has just now been created because you clicked on it!" but the main thing is for it to be immediately visually obvious the difference between hitting an existing promise and creating one, hence something drastic like a different background color.)

Promises are gray briefly when they become overdue

They're supposed to be pink (or like on fire) when they're overdue but there seems to be a short window after the due date when they're gray.

I need to pin down the repro steps but maybe it will end up being obvious what's up in the code.

"by today" is changed to last night midnight if you touch the edit form

Steps to Reproduce

  1. http://alice.commitsto.review/repro_this_bug_by_today
  2. optionally add a note or some other small edit but you can also skip this step altogether
  3. click SAVE

What I Expect

It leaves tdue set to tonight at 11:59pm

What Happens

It changes tdue to midnight last night (so technically still "today" but 24 hours sooner than the user wanted).

Hints and Comments

This is looks like potentially a dup of or at least related to Issue #160

Don't let dates parse as in the past

Repro

  1. Make sure it's before 4pm pacific, 7pm eastern, ie, before midnight UTC
  2. Click http://alice.commitsto.review/foo/by/today

Expect

A deadline of midnight tonight. (Or ideally 11:59:59pm so the calendar date is still in fact today.)

What Happens

A deadline of midnight yesterday night.

Commentary

There are 2 things going on:
(1) A bug or misconfiguration that's making it interpret "today" in terms of UTC.
(2) Parsing "today" as the beginning of the day instead of the end of the day.

Those two things sometimes cancel each other out and produce the correct behavior, which was pretty confusing for a while!

So if we just fix (1) then we're going to consistently get deadlines at 24 hours (well, 23h59m59s) sooner than we want. Then we just need to add the 86399 seconds to any deadline that doesn't specify a specific time of day.

I see from http://blog.metamorphium.com/Sherlock/ that the date parser tells you whether it interpreted it as "all day" or not. So we need to add 86399 seconds if so. Or maybe use endDate instead of startDate?

Store all promises on GET

As part of the MVP, let's actually create and store a promise immediately when the GET request happens ...

[UPDATE: Originally I was thinking we'd only show confirmed promises by default. See my new comment for my latest thinking.]

Normally we can show only confirmed promises but we'll have the rest stored just in case. That's also a good safeguard against bugs. We really don't want to accidentally drop any promises on the floor, even at this stage (which is also why I'm fastidiously pasting every incoming promise in data/seed.js for now).

Don't create promises for "robots.txt" etc

The following are examples of URLs that get hit by bots and things that shouldn't generate commitments:

/robots.txt
/apple-touch-icon.png
/apple-touch-icon-120x120.png
/apple-touch-icon-120x120-precomposed.png

UPDATE: Mistakenly referred to this as blacklisting URLs initially. We just want to serve some versions of those so the create-promise route doesn't fire.

Serve a custom 404 page

Create a 404.handlebars template and serve it up instead of the text-only response here: https://github.com/beeminder/iwill/blob/master/app/middleware.js#L38

Something like:

<h1>
	404 Not Found
</h1>

<h3>
	Were you trying to create a commitment?
</h3>

<p>
	Right now the preferred format is like
	<tt>alice.commits.to/do_the_thing</tt>
</p>

<p>
	Please avoid any other special characters right now.
	Things are a bit in flux with the URL format!
	And there are a couple special characters that will cause things to break horribly.
</p>

Domain name agnosticism

from email with @dreeves:

i think there should be one canonical name that everything redirects to.
i propose "commits.to" as the canonical one. in fact, it's possible
that my friend sergii will want "promises.to" (he owns that domain and
i own "commits.to") to point to a different implementation of this.

Make promise voiding work

Setting void=true should mean the promise doesn't show up in the user's gallery and isn't included in the reliability calculations. You can still see the promise if you go straight to its URL but otherwise it's invisible.

(Exception which could be a separate Issue: A special route like alice.commits.to/voided to see all of a user's voided promises.)

Why void a promise? Maybe it became moot or impossible or something and you and the person you promised agree that it doesn't make sense for it to count against you. You should probably add notes in the note field for posterity about why you're voiding it.

Difference between voiding and deleting

Voiding is a bit like soft-deleting. It retains the promise in the database and keeps the URL reserved. The problem with deleting with our crazy create-on-GET scheme is that if the URL is out there anywhere then you're in for an annoying surprise when the promise comes back to life.

Deleting should possibly be not even possible. But there may be use cases for it. Maybe you created an erroneous URL that no one else saw and so you know it won't get GET'd later. Or maybe if it did get GET'd later you'd want it created as a fresh promise as if its sordid history never even existed.

I think we should make more decisions about deleting before we accept more beta users but in the meantime we can close this Issue when the voiding feature works!

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.