Instagram node #27

zobalogh · 2014-09-29T16:02:24Z

This is my attempt on the Instagram node. Current example is showing the query node. Comments are welcome. I'll squash commits once we get this work into a pullable state. Thanks

The pull request is now ready @knolleary .

zobalogh · 2014-09-29T16:04:58Z

instagram/instagram.js

+                                var scoped_msg = clone(msg);
+
+
+                                http.get(scoped_url, function(res) {


@knolleary , what do you think about using this http.get and specifying the 'binary' nature of the buffer? It will be deprecated in the future versions of node. However it isn't deprecated yet and is an easy way to buffer into memory.

There's a half-baked "buffer to disk" solution commented out. I'm not sure how I could trigger request to download to memory.

What's the best way forward here?

Try request that we are already using elsewhere. The request callback has three parameters, the third being the body as a Buffer.

zobalogh · 2014-10-01T14:00:24Z

@knolleary I think the pull request is now in a state where you might want to take a look at it. Comments are welcome! The pull request has the initial requirements in it. Adding functionality such as location based information is to be defined and added later in a separate pull request. Thanks

zobalogh · 2014-10-01T14:46:27Z

@knolleary pull request is in [wip] state again as I realised that currently when selecting the "photos uploaded by user" feature, it fetches all images uploaded by the user, ever. I'll need to fix this before we drop the node. Will let you know when this is fixed.

zobalogh · 2014-10-01T16:40:50Z

@knolleary The pull request is now ready for review. Since my last comment I:

discovered that the problem mentioned above was due to the inject node injecting a message into the Instagram node before it has initialized itself successfully. The uninitialized node defaults to the "user has not uploaded any known media" mode and thus it fetches everything. Now the node drops all incoming messages and ignores them if not initialized. It also warns the user. This is the simplest solution I could think of.
As discussed, I simplified the output so now msg.filename doesn't exist any more and the node doesn't know anything about files/paths or anything related to a file system. For v1, this is the desired behaviour and additional message payload/property is to be determined later.

Thanks!

zobalogh · 2014-10-02T16:14:33Z

instagram/instagram.js

+        RED.nodes.addCredentials(node_id,credentials);
+    });
+
+    RED.httpAdmin.get('/instagram-credentials/auth/redirect', function(req, res) {


@hindessm , I managed to lose your comments when overwriting the pull request's branch with my local one. I made a response on your comment about the redirect url. I deliberately decided to choose a URI that's different from our current convention as I disagree with it. The user is setting up the "redirect URL" in most frameworks. Having redirect in the URL gives confirmation to the user that they're doing the right thing. Having callback here would just confuse the user as they'll start to wonder what that means... Yes, it's a callback from OUR perspective but a redirect from the user's perspective. So I propose to change all of them to redirect. Sorry, it stays as is for the time being!

Interesting. However, the fitbit app configuration calls it "Callback URL" though so you shouldn't change that one and I think we want to be consistent. Since oauth calls it the oauth callback I suspect that continuing to use callback for consistency makes most sense.

Luckily I don't think users pay too much attention to the text of the URL they are cutting and pasting and you can write the accurate field name in the configuration hint to make sure they know exactly which field it needs to be pasted into.

OK, I'll change it then! I would still rather use my URI and just simply tag the function, if consistency is key. What do you think about adding a tag to each such callback?

What do you mean by 'tag the function'? As has been said, the urls were picked based on what oauth calls them. I don't see a compelling reason for this node to do something different.

I was offering the option of having something in the path that actually
resembles what the given service calls the OAuth callback. This however
means that the callback URL will be different in some nodes. In order to
match them up, we could add a "callback" tag to all these express functions.

I'm considering that the different name (matching the service's on naming
convention) is for the user's convenience as in the instagram case, it's
called a "rediredt url" and if the user sees "redirect" in the URL, they're
more confident that they're copying the right field into the right place.

Mark says that as long as the instructions are clear, it doesn't matter
what the callback uri is and he'd therefore leave it as callback. I would
do the option I sketched up above.

Thanks

On 3 October 2014 12:05, knolleary notifications@github.com wrote:

In instagram/instagram.js:

res.redirect(Url.format({

protocol: 'https',

hostname: 'api.instagram.com',

pathname: '/oauth/authorize/',

query: {

client_id: credentials.client_id,

redirect_uri: credentials.redirect_uri,

response_type: "code",

state: node_id + ":" + credentials.csrfToken

}

}));

RED.nodes.addCredentials(node_id,credentials);

});

RED.httpAdmin.get('/instagram-credentials/auth/redirect', function(req, res) {

What do you mean by 'tag the function'? As has been said, the urls were
picked based on what oauth calls them. I don't see a compelling reason for
this node to do something different.

—
Reply to this email directly or view it on GitHub
https://github.com/node-red/node-red-web-nodes/pull/27/files#r18389821.

knolleary · 2014-10-03T13:12:40Z

The icon should not be the actual Instagram icon. Needs to conform to our icon requirements: http://nodered.org/docs/creating-nodes/appearance.html

zobalogh · 2014-10-03T13:21:21Z

I did want to use our conventions, however Instagram's terms and conditions
don't allow that. You can either use their icon (can pick black and white
or colour one) or nothing at all.
2. Brand Assets.

Use of Instagram's brand assets (e.g., the Instagram name and logo) is
governed by Instagram's Brand Guidelines
http://help.instagram.com/304689166306603, which are incorporated herein
by reference.

https://help.instagram.com/304689166306603/

which says:

Don’t:

Use the Instagram brand in a way that implies partnership, sponsorship
or endorsement
Combine any part of the Instagram brand with your name, marks, designs
or generic terms
Use trademarks, names, domain names, usernames, logos or other content
that imitate or could be confused with Instagram
Present Instagram in a way that makes it the most distinctive or
prominent feature of what you’re creating
Use any icons, images or trademarks to represent Instagram other than
what is found on this page
Copy the Instagram look and feel

On 3 October 2014 14:12, knolleary notifications@github.com wrote:

The icon should not be the actual Instagram icon. Needs to conform to our
icon requirements: http://nodered.org/docs/creating-nodes/appearance.html

—
Reply to this email directly or view it on GitHub
#27 (comment)
.

knolleary · 2014-10-03T13:29:51Z

Okay, but that doesn't mean we abandon our visual look. We should use a generic photo/picture icon.

zobalogh · 2014-10-03T13:33:20Z

I think that's exactly what their terms don't allow us. Do you read their
T&C differently to me?

Don't
Use any icons, images or trademarks to represent Instagram other than
what is found on this page

On 3 October 2014 14:29, knolleary notifications@github.com wrote:

Okay, but that doesn't mean we abandon our visual look. We should use a
generic photo/picture icon.

—
Reply to this email directly or view it on GitHub
#27 (comment)
.

knolleary · 2014-10-03T13:36:10Z

From that same page:

Where color is limited, use the Glyph Logo in any color

So, please use their glyph icon in white.

zobalogh · 2014-10-03T13:41:30Z

OK! Do you have any problems with the current colour of the node? I used a
colour picker on their website's banner so the node's blue is in Instagram
colours!

On 3 October 2014 14:36, knolleary notifications@github.com wrote:

From that same page:

Where color is limited, use the Glyph Logo in any color

So, please use their glyph icon in white.

—
Reply to this email directly or view it on GitHub
#27 (comment)
.

knolleary · 2014-10-03T13:44:36Z

I'd suggest something more like #889FB3 - similar in tone, just a bit lighter and in-keeping with our muted palette.

zobalogh · 2014-10-03T13:50:52Z

OK, thanks, I'll change the icon and the colour and push the new branch
with it.

On 3 October 2014 14:44, knolleary notifications@github.com wrote:

I'd suggest something more like #889FB3 - similar in tone, just a bit
lighter and in-keeping with our muted palette.

—
Reply to this email directly or view it on GitHub
#27 (comment)
.

zobalogh · 2014-10-03T15:18:55Z

@knolleary done

knolleary · 2014-10-06T10:03:57Z

instagram/instagram.html

+        inputs:0,
+        outputs:1,
+        icon: "instagram.png",
+        align: "right",


Input nodes should not have align set to right - it defaults to left, so you can remove this entry.

knolleary · 2014-10-06T10:31:43Z

Some comments:

can you put the Input node above the query node in the .html file - that affects the order they appear in the palette - input should be above query
can you and @hbeeken sync-up the appearance of the tip regarding callback url - this and the 4sq node look different. Don't mind which you two pick... just pick one.
The options in the edit dialog could do with some tweaks
- 'Type'
  - suggest changing the text of the options to simply "User photos" and "Liked photos"
- 'Output'
  - suggest changing the text of the options to: "the image as a buffer", "the image url"
Changing the poll interval to 15 minutes - that's consistent with our other polling nodes and is more respectful of the api limits.

hbeeken · 2014-10-06T11:52:20Z

@knolleary: regarding your comment "can you and @hbeeken sync-up the appearance of the tip regarding callback url - this and the 4sq node look different. Don't mind which you two pick... just pick one.", we've decided to go with Zoltan's wording as that sounds more general. However, this means that the google calendar node ( @hindessm ) needs updating to the same wording.

zobalogh · 2014-10-06T12:42:20Z

@knolleary I made all the changes you required above. I re-worded the node description and photos are now consistently called "photos". I also added a new paragraph to the node's description (last paragraph) to mention that the node currently only works with photos and not with videos.

knolleary · 2014-10-06T13:43:47Z

The help text talks about setting msg.filename, which I don't believe is still in the code, so needs removing.

I don't think the query node is entirely clear about what it returns - what query does it actually do? (rhetorical question... don't need an answer here... the help needs updating to make it clear)

zobalogh · 2014-10-06T15:18:35Z

@knolleary I made the changes to the help txt as suggested by you. Thanks! I also updated the icon to be closer to the homepage's guidelines in terms of its dimension.

zobalogh · 2014-10-07T13:20:55Z

@knolleary All changes suggested by @hindessm are now done.

knolleary · 2014-10-08T08:41:58Z

instagram/instagram.js

+    var ig = require('instagram-node').instagram();
+
+    // needed for normal operation
+    var clone = require('clone');


Do you still need to depend on clone? I don't see it used anywhere.

zobalogh · 2014-10-08T13:00:02Z

instagram/instagram.js

+            ig.use({ access_token: credentials.access_token });
+            ig.user('self', function(err, result, remaining, limit) { // warning, this is not the same as the other result or res!
+                if (err) {
+                    this.warn('Instagram node has failed to fetch the username : '+err);


@knolleary, marked my node as WIP again as it looks like this.warn is a bad choice for the auth process as it's this is undefined at this point! Noticed this when writing nock tests!

I think a more appropriate error message would be maybe to send a warning to res.send(). I'll fix this.

Instagram node

zobalogh reviewed Sep 29, 2014
View reviewed changes

zobalogh force-pushed the instagram-node-dev branch 3 times, most recently from 61f1e89 to b3b99fd Compare October 1, 2014 13:57

zobalogh changed the title ~~[wip]Instagram node dev~~ Instagram node Oct 1, 2014

zobalogh changed the title ~~Instagram node~~ [wip]Instagram node Oct 1, 2014

zobalogh force-pushed the instagram-node-dev branch from b61eec6 to 998ea5e Compare October 1, 2014 16:20

zobalogh changed the title ~~[wip]Instagram node~~ Instagram node Oct 1, 2014

zobalogh changed the title ~~Instagram node~~ [wip]Instagram node Oct 1, 2014

zobalogh force-pushed the instagram-node-dev branch from 11fec91 to c955b98 Compare October 1, 2014 16:37

zobalogh changed the title ~~[wip]Instagram node~~ Instagram node Oct 1, 2014

zobalogh force-pushed the instagram-node-dev branch from cb1953b to 6dfcead Compare October 2, 2014 16:07

zobalogh reviewed Oct 2, 2014
View reviewed changes

zobalogh force-pushed the instagram-node-dev branch 2 times, most recently from decd380 to 7900296 Compare October 3, 2014 10:26

zobalogh force-pushed the instagram-node-dev branch from b091049 to f26984b Compare October 3, 2014 15:13

knolleary reviewed Oct 6, 2014
View reviewed changes

zobalogh force-pushed the instagram-node-dev branch 3 times, most recently from 9e2f517 to da61040 Compare October 6, 2014 10:28

zobalogh force-pushed the instagram-node-dev branch from da61040 to 3ce9838 Compare October 6, 2014 10:52

zobalogh force-pushed the instagram-node-dev branch from 3ce9838 to f11daf4 Compare October 6, 2014 12:40

zobalogh force-pushed the instagram-node-dev branch from f11daf4 to d4cd22c Compare October 6, 2014 15:17

zobalogh force-pushed the instagram-node-dev branch from d4cd22c to 9ec9b85 Compare October 7, 2014 13:20

knolleary reviewed Oct 8, 2014
View reviewed changes

zobalogh force-pushed the instagram-node-dev branch 2 times, most recently from 785499e to 813226f Compare October 8, 2014 09:51

zobalogh changed the title ~~Instagram node~~ [wip]Instagram node Oct 8, 2014

zobalogh reviewed Oct 8, 2014
View reviewed changes

zobalogh force-pushed the instagram-node-dev branch from 7cff243 to 7e99d30 Compare October 8, 2014 13:48

zobalogh changed the title ~~[wip]Instagram node~~ Instagram node Oct 8, 2014

Adding Instagram nodes

c55eea2

zobalogh force-pushed the instagram-node-dev branch from 33a4c8a to c55eea2 Compare October 8, 2014 15:48

knolleary added a commit that referenced this pull request Oct 10, 2014

Merge pull request #27 from zobalogh/instagram-node-dev

8098f7b

Instagram node

knolleary merged commit 8098f7b into node-red:master Oct 10, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Instagram node #27

Instagram node #27

zobalogh commented Sep 29, 2014

zobalogh Sep 29, 2014

hindessm Sep 29, 2014

zobalogh commented Oct 1, 2014

zobalogh commented Oct 1, 2014

zobalogh commented Oct 1, 2014

zobalogh Oct 2, 2014

hindessm Oct 3, 2014

zobalogh Oct 3, 2014

knolleary Oct 3, 2014

zobalogh Oct 3, 2014

knolleary commented Oct 3, 2014

zobalogh commented Oct 3, 2014

knolleary commented Oct 3, 2014

zobalogh commented Oct 3, 2014

knolleary commented Oct 3, 2014

zobalogh commented Oct 3, 2014

knolleary commented Oct 3, 2014

zobalogh commented Oct 3, 2014

zobalogh commented Oct 3, 2014

knolleary Oct 6, 2014

knolleary commented Oct 6, 2014

hbeeken commented Oct 6, 2014

zobalogh commented Oct 6, 2014

knolleary commented Oct 6, 2014

zobalogh commented Oct 6, 2014

zobalogh commented Oct 7, 2014

knolleary Oct 8, 2014

zobalogh Oct 8, 2014

		var scoped_msg = clone(msg);


		http.get(scoped_url, function(res) {

Instagram node #27

Instagram node #27

Conversation

zobalogh commented Sep 29, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zobalogh commented Oct 1, 2014

zobalogh commented Oct 1, 2014

zobalogh commented Oct 1, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knolleary commented Oct 3, 2014

zobalogh commented Oct 3, 2014

knolleary commented Oct 3, 2014

zobalogh commented Oct 3, 2014

knolleary commented Oct 3, 2014

zobalogh commented Oct 3, 2014

knolleary commented Oct 3, 2014

zobalogh commented Oct 3, 2014

zobalogh commented Oct 3, 2014

Choose a reason for hiding this comment

knolleary commented Oct 6, 2014

hbeeken commented Oct 6, 2014

zobalogh commented Oct 6, 2014

knolleary commented Oct 6, 2014

zobalogh commented Oct 6, 2014

zobalogh commented Oct 7, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment