Issue-89 - Add support for arrays in query parameters #98

pavelpantus · 2016-04-07T19:28:04Z

Possible solution if I got the meaning right.

chrismaddern · 2016-04-07T19:31:45Z

DeepLinkKit/Categories/NSString+DPLQuery.h

+ Removes array literal ('[]') from the string
+ @return String without array literal ('[]')
+ */
+- (NSString *)DPL_removeArrayLiteral;


Convention would indicate:
DPL_stringByRemovingArrayLiteral or something similar

Thanks for noticing, will correct it shortly.

On Thursday, April 7, 2016, Chris Maddern [email protected] wrote:

In DeepLinkKit/Categories/NSString+DPLQuery.h
#98 (comment):

+- (BOOL)DPL_containsArrayLiteral;
+
+
+/**

Checks if string contains key with array literal

@return YES if string contains key with '[]' attached (e.g. "key[]"),

NO otherwise (e.g. "key")

/
+- (BOOL)DPL_containsArrayLiteralWithKey:(NSString *)key;
+
+
+/*

Removes array literal ('[]') from the string

@return String without array literal ('[]')

*/
+- (NSString *)DPL_removeArrayLiteral;

Convention would indicate:
DPL_stringByRemovingArrayLiteral or something similar

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/button/DeepLinkKit/pull/98/files/7cebc8717560a0a64fb675df53f041ef2e2d7acb#r58930895

Best regards,
Pavel Pantus

pavelpantus · 2016-04-07T23:06:45Z

That's really cool test failure:
expected: beers[]=stout&beers[]=ale&liquors[]=vodka&liquors[]=whiskey
got: liquors[]=vodka&liquors[]=whiskey&beers[]=stout&beers[]=ale

(Different ordering in NSDictionary)

Test passes on my machine though :)

pavelpantus · 2016-04-07T23:40:11Z

SampleApps/SenderDemo/ActionData/DPLActionDataSource.m

@@ -74,4 +75,14 @@ - (DPLDemoAction *)logHelloWorldAction {
    return action;
 }

+
+- (DPLDemoAction *)logShoppingList {
+    DPLMutableDeepLink *link = [[DPLMutableDeepLink alloc] initWithString:@"dpl://shoppinglist?list%5B%5D%3DBread&list%5B%5D%3DCandies&list%5B%5D%3DWine&list%5B%5D%3DBeer"];


I was wondering if here should be easier way to initialize deep link with URLs of such kind?

pavelpantus · 2016-04-08T17:54:47Z

@wessmith @chrismaddern Hey guys, wanted to ask for your feedback.
Is it the right direction or I missed something important here? Thanks!

wessmith · 2016-04-08T18:00:19Z

Hi @pantuspavel this is looking good 😄 but I haven't had a chance to fully go through it.

wessmith · 2016-04-08T18:03:34Z

Tests/Categories/NSString_DPLQuerySpec.m

+        NSString *queryStringToMatch;
+        NSString *queryStringPartBeers = @"beers[]=stout&beers[]=ale";
+        NSString *queryStringPartLiquors = @"liquors[]=vodka&liquors[]=whiskey";
+        if ([[params allKeys][0] isEqualToString:@"beers"]) {


I think we should probably have a suitable default here such as ordered ascending that way we can expect a consistent result.

Similarly, when parsing a url, I would expect to have the parameter order preserved so re-serialization mirrors the original url though that can't really happen in the category (#15) without more information.

For example:

NSURL *url = [NSURL URLWithString:@"dpl://something/?liquors[]=vodka&liquors[]=whiskey&beers[]=stout&beers[]=ale"]; DPLDeepLink *link = [[DPLDeepLink alloc] initWithURL:url]; link.URL.query // should serialize as => @"liquors[]=vodka&liquors[]=whiskey&beers[]=stout&beers[]=ale"

@wessmith Just making sure I understand that correctly: you are talking here about DPLMutableDeepLink not DPLDeepLinkright? (DPLDeepLink returns previously set URL without any serialization work).

You're right. Yeah.

pavelpantus · 2016-04-08T18:14:41Z

Hey @wessmith,
That definitely makes sense! I'll make this happen.
Thanks and sorry for pushing, was very curious to get your feedback! 😄

wessmith · 2016-04-08T18:18:16Z

@pantuspavel If you want to tackle #15 I'd love to hear your thoughts on how you would approach it before you get too far into it :)

pavelpantus · 2016-04-08T18:34:18Z

@wessmith sure,
I'm thinking to sort parameters first and then serialize them in:

[NSString DPL_queryStringWithParameters:(NSDictionary *)parameters];

This way we always get the same order in the output string.

wessmith · 2016-04-08T18:46:37Z

That works. Then we can tackle #15 later.

pavelpantus · 2016-04-08T18:58:01Z

@wessmith ooh, I just got what #15 is about.
Yeah, you're right. We'll need more info here.
I'll think about solution, but first will commit the consistent ordering.

pavelpantus · 2016-04-08T19:47:42Z

What do you think of using some kind of ordered dictionary(e.g. M13OrderedDictionary - not sure about this one though. Last commit - Oct. 2015)

wessmith · 2016-04-08T19:54:37Z

We don't want to add any dependencies. Perhaps we pass in an NSArray or NSOrderedSet of sorted keys to an overloaded [NSString DPL_queryStringWithParameters:sortedKeys:]

pavelpantus · 2016-04-08T20:21:46Z

Yeah, that was my second thought -- dependencies.
Your solution sounds good to me.
Thanks for the idea.

On Friday, April 8, 2016, Wes Smith [email protected] wrote:

We don't want to add any dependencies. Perhaps we pass in an NSArray or
NSOrderedSet of sorted keys to an overloaded [NSString
DPL_queryStringWithParameters:sortedKeys:]

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#98 (comment)

Best regards,
Pavel Pantus

lucasecf · 2016-06-21T09:53:14Z

Hey guys, any plan to merge this?

chrismaddern · 2016-12-06T19:53:31Z

@wessmith @pantuspavel what's the action here? Would love to get this merged!

wessmith · 2017-03-31T14:53:58Z

@pantuspavel can you have a look at this and make sure its up to date and all comments are resolved

Issue-89 - Add support for arrays in query parameters

7cebc87

chrismaddern reviewed Apr 7, 2016
View reviewed changes

pavelpantus added 2 commits April 7, 2016 18:45

Should not parse path components as a query fix, minor rearrangements.

3b6c0e8

Removing unneeded include

3899a9e

Fixing test by matching the NSDictionary ordering

b8b6ab5

pavelpantus reviewed Apr 7, 2016
View reviewed changes

wessmith reviewed Apr 8, 2016
View reviewed changes

pavelpantus added 2 commits April 8, 2016 15:05

Consistent ordering on parameters serialization.

3fd49d9

More convinient way to initialize deep link

01ef2e1

Preserving query parameters order.

b7bd8f5

wessmith self-requested a review March 31, 2017 14:53

wessmith assigned pavelpantus Mar 31, 2017

wessmith added the selected for review label Mar 31, 2017

jmcgill closed this Jan 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue-89 - Add support for arrays in query parameters #98

Issue-89 - Add support for arrays in query parameters #98

pavelpantus commented Apr 7, 2016

chrismaddern Apr 7, 2016

pavelpantus Apr 7, 2016

pavelpantus commented Apr 7, 2016

pavelpantus Apr 7, 2016

pavelpantus commented Apr 8, 2016

wessmith commented Apr 8, 2016

wessmith Apr 8, 2016

pavelpantus Apr 8, 2016

wessmith Apr 8, 2016

pavelpantus commented Apr 8, 2016

wessmith commented Apr 8, 2016

pavelpantus commented Apr 8, 2016

wessmith commented Apr 8, 2016

pavelpantus commented Apr 8, 2016

pavelpantus commented Apr 8, 2016

wessmith commented Apr 8, 2016

pavelpantus commented Apr 8, 2016

lucasecf commented Jun 21, 2016

chrismaddern commented Dec 6, 2016

wessmith commented Mar 31, 2017

Issue-89 - Add support for arrays in query parameters #98

Issue-89 - Add support for arrays in query parameters #98

Conversation

pavelpantus commented Apr 7, 2016

chrismaddern Apr 7, 2016

Choose a reason for hiding this comment

pavelpantus Apr 7, 2016

Choose a reason for hiding this comment

pavelpantus commented Apr 7, 2016

pavelpantus Apr 7, 2016

Choose a reason for hiding this comment

pavelpantus commented Apr 8, 2016

wessmith commented Apr 8, 2016

wessmith Apr 8, 2016

Choose a reason for hiding this comment

pavelpantus Apr 8, 2016

Choose a reason for hiding this comment

wessmith Apr 8, 2016

Choose a reason for hiding this comment

pavelpantus commented Apr 8, 2016

wessmith commented Apr 8, 2016

pavelpantus commented Apr 8, 2016

wessmith commented Apr 8, 2016

pavelpantus commented Apr 8, 2016

pavelpantus commented Apr 8, 2016

wessmith commented Apr 8, 2016

pavelpantus commented Apr 8, 2016

lucasecf commented Jun 21, 2016

chrismaddern commented Dec 6, 2016

wessmith commented Mar 31, 2017