Widgets work with both ~users and Teams

[mattbk]

I took a crack at aligning the default widget code with the new public.json output from Gratipay.com. Comments are welcome; I'm sure there are ways to break what I've done.

[chadwhitacre]

Nice! I've made a Review label in grtp.co so we could apply it here. :)

Anyone up for reviewing this? If not I'll get to it when I can ...

[mattbk]

Should I worry about these checks failing?

[chadwhitacre]

@mattbk Yes. It looks like our linter ain't happy with your formatting. ;-)

[mattbk]

Okay, I see it now.

lib/v1/api.js 131 | text('receiving', (data.taking)? '$' + data.taking : '$' + data.receiving); ^ [W015] Expected 'text' to have an indentation at 17 instead at 21. 141 | } ^ [W015] Expected '}' to have an indentation at 17 instead at 21.

[mattbk]

I think tests are based on the old public.json and are failing because I downloaded new versions of the test public.json files?

[mattbk]

Looking at these files in order?

Files: test/test_custom-widget-legacy.js, test/test_custom-widget.js, test/test_default-widget-anonymous.js, test/test_default-widget-legacy.js, test/test_default-widget.js, test/test_expose-api-to-iframe.js, test/test_giving-widget-anonymous.js, test/test_giving-widget-legacy.js, test/test_giving-widget.js -> test

[mattbk]

I think it's failing on this part of api.js for certain tests.

  // Set up links and username before requesting public.json
            link('profile-link', gratipayURI + encodeURIComponent(username) + '/');
            link('link', gratipayURI);
            text('username', username);
            text('identity', 'I');

[mattbk]

That works. Hot.

[chadwhitacre]

/me waiting for Travis to run green ...

[mattbk]

/me waiting for Travis to run green ...

Hold your horses, I have to understand how tests work first.

[chadwhitacre]

!m @mattbk

[mattbk]

It looks like only giving can be anonymous now, not receiving. That doesn't help my brain work better, though.

[mattbk]

Previous commit/revert tells me the issue is in api.js, not v1.js.

This must be fascinating to watch.

[chadwhitacre]

@mattbk at https://github.com/gratipay/inside.gratipay.com/issues/280#issuecomment-185306575:

I think I've gotten the base functionality down (at least for the limited scope of the PR), but I'm not sure how to make Travis okay with it. I think the issue is looking for ~user data, but getting team data from public.json and failing.

Any insight would be appreciated.

[chadwhitacre]

lol

[chadwhitacre]

/me figuring out how to run tests for grtp.co ...

[chadwhitacre]

.travis.yml doesn't explicate ...

[chadwhitacre]

For projects using npm, Travis CI will execute

npm test

to run your test suite.

https://docs.travis-ci.com/user/languages/javascript-with-nodejs#Default-Test-Script

[chadwhitacre]

Okay, I get the same result locally that I'm seeing in the latest Travis build here on this PR.

[chadwhitacre]

Let's see if tests pass on master ...

[chadwhitacre]

Yes!

[chadwhitacre]

Alright, looks like all six failing tests are failing in the same way: they're expecting a readystatus of ready, but are seeing loading api.js step 3 instead.

@mattbk Why do you think it breaks here?

One practice that I find helpful is to isolate a single test case, such that, even though six are failing here, I only focus on one for debugging purposes. Do you know how to do that with npm test?

Let's think in terms of the default-widget-anonymous: readystatus should be ready test ...

Given the observed behavior thus far, what's your hypothesis about why that test is failing? What's a test we can perform to validate your hypothesis? Like, can we console.log some variables at some point to see what they are? (I'm not sure what interactive debugging facilities are available for node).

Remember, debugging is Science Lite™! :microscope: :older_man:

[mattbk]

@mattbk Why do you think it breaks here?

I added those status updates with step 1, step 2, step 3. It gets to step 3 and then (I think) goes into widget.setAttribute('data-gratipay-username', username); and doesn't end up setting the status as ready. I think this is because username is empty for some reason.

I could be way off on that. I don't know how to do much of anything relating to actual frameworks, but I can probably learn the basics of npm test if that will let me check code without going through the commit process each time.

[chadwhitacre]

I think this is because username is empty for some reason.

A hypothesis!

I don't know how to do much of anything relating to actual frameworks, but I can probably learn the basics of npm test if that will let me check code without going through the commit process each time.

Have you been editing on GitHub through their web editor? Or are you set up to edit locally? Do you have npm installed locally? What operating system are you running?

[mattbk]

Locally, OSX. I'll have to look for npm when I get in front of that computer, but I went through the whole install/local build in the readme.

[chadwhitacre]

Okay, cool. Me, too. Do you use brew? If not I believe npm comes with the package installer on nodejs.org.

[chadwhitacre]

Yup.

[mattbk]

Yes, I use brew. Will look at this fresh locally when I get a chance. Thanks for taking a look.

[chadwhitacre]

Thanks for working on it! Once the tests are passing I'll give this a proper review. I need to understand what the purpose is here and also what our widget code is already like. :)

[mattbk]

For fun while simulations are running, installed Chocolatey and then npm in Windows 7. Then had to initialize/start local grunt with npm install in the grtp.co directory, then successfully ran npm test and got similar output. So there's a more streamlined approach to testing.

I don't know what it all means, but I can make it work.

[chadwhitacre]

Ah, nice! !m @mattbk

Check the npm docs, you should be able to run just a single test pretty easily.

[mattbk]

It's going into (or maybe not even going into) Gratipay.widgets.apply() and not coming out again, so data-gratipay-readystatus is never set as ready.

[mattbk]

What a difference me accidently deleting a line makes.

widget.setAttribute('data-gratipay-username', username); was gone, so username wasn't making it from one function to another. Things went faster once I realized I could use the JS console in Chrome.

[chadwhitacre]

Nice! Scientific progress goes boink! :-)

[mattbk]

I ended up merging trailing-zero into this branch because this is what it was based on. Next up is a full review by me of what I actually did.

[mattbk]

I'm satisfied with the final product; it's a heck of a mess of commits, though. :wolf:

@whit537, @techtonik, any comments?

[aandis]

it's a heck of a mess of commits

Github now allows squash and merge commits! :dancer:

[chadwhitacre]

As I read it, this PR tries to treat the data-gratipay-username of widgets-in-the-wild as either a ~user username or a Team slug.

1. If it's a username, we use that to construct the profile link, and expect that receiving won't be present but taking will be.
2. If it's a slug, we use that to construct the profile link, and expect receiving to be present.

I'm not sure that's the right approach to take, both because it introduces tech debt into our widget code ("receiving" in this PR no longer means "receiving," it means "receiving or taking"), and because it's not as true to our new data model as it could be. I think a better approach will be to let data-gratipay-username continue to only mean ~user username, and write new widget code for Teams. See https://github.com/gratipay/grtp.co/issues/103#issuecomment-207886406 for further discussion.

I propose that we whittle this PR down to just the test changes that make sense to keep, and pick up with additional changes in new PRs.

[mattbk]

I propose that we whittle this PR down to just the test changes that make sense to keep, and pick up with additional changes in new PRs.

I can work on that, unless someone else wants to jump in.

[mattbk]

just the test changes

I might need clarification. You mean make this PR about ~users and build a new PR for teams?

[chadwhitacre]

I actually meant to keep the extra info in the test strings like below, and make new PRs for ~user and Team changes.

[mattbk]

Got it. Now I get to learn how to clone my PR without losing things.

[mattbk]

Shuttering this pull request because it changes v1 widgets, which is not the plan at https://github.com/gratipay/grtp.co/issues/103#issuecomment-215227273.

Keeping the branch in place until I grab what I want for a real v2.