-
Notifications
You must be signed in to change notification settings - Fork 13
fix Users.Get(int[] ids, ...), add "Get Users" to console scene #80
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Phew that was a lot. It would probably worth working on branches next time so the PR don't get "polluted" with other work.
If you update the CHANGELOG.md with the changes you've made to User.Get
and DataStore.GetKeys
(adding pattern), I'll merge the PR.
Assets/Tests/Console/ConsoleTest.cs
Outdated
@@ -62,6 +64,18 @@ public void SignOut() | |||
AddConsoleLine(string.Format("Sign Out {0}.", isSignedIn ? "Successful" : "Failed")); | |||
} | |||
|
|||
public void GetUsersById() { | |||
var ids = ParseIds(userIdsField.text); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The initial goal of the console was to give people an easy way in but I've never released it officially (at least not outside of the repo), but to keep things consistent, it would be great to have AddConsoleLine("Get Users by ID. Click to see source.")
@@ -343,6 +352,8 @@ void Start() | |||
userNameField.text = settings.user; | |||
userTokenField.text = settings.token; | |||
} | |||
userIdsField.onValidateInput += ValidateIdList; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat 👌
{ | ||
var parameters = new Dictionary<string, string>(); | ||
parameters.Add(Constants.API_USERS_FETCH, string.Join(",", ids.Select(id => id.ToString()).ToArray())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Err that was a really silly mistake. Nice work.
@@ -119,11 +93,25 @@ public static void Delete(string key, bool global, Action<bool> callback = null) | |||
/// <summary> | |||
/// Gets the list of available keys in the DataStore. | |||
/// </summary> | |||
/// <param name="global">A boolean indicating whether the keys are global (<c>true<c/>) or private to the user (<c>false<c/>).</param> | |||
/// <param name="global">A boolean indicating whether the keys are global (<c>true</c>) or private to the user (<c>false</c>).</param> | |||
/// <param name="callback">A callback function accepting a single parameter, a list of key names.</param> | |||
public static void GetKeys(bool global, Action<string[]> callback) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one on making it backward compatible 👍
yeah sorry, the pull request was only for the "get users"-fix. I forgot to switch to a dedicated branch. I will update the PR today or tomorrow so we can merge it soon. |
PR ready to merge |
Well done! Thanks for your help. |
This fixes issue #79 and also adds a "Get Users" function to the Console scene.