Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added the possibility to edit showcases #207

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Added the possibility to edit showcases #207

Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
087c0f0
Added the possibility to edit showcases
NetroScript Jul 19, 2018
9b1d40c
Added the possibility to edit showcases
NetroScript Jul 19, 2018
a5a03f1
Fixing potential error
NetroScript Jul 20, 2018
0f9ae26
Removed dependency of param
NetroScript Jul 20, 2018
de4c377
Ensures the array index for the submitted settings.
NetroScript Jul 20, 2018
a8d9f16
Improved readability of `showcaseconfig`
NetroScript Jul 20, 2018
a9264f2
Replaced t with type and added delay to the `ajaxsetshowcaseconfig` r…
NetroScript Jul 20, 2018
72b57d4
Improved behaviour of callbacks when a request to change a single ite…
NetroScript Jul 22, 2018
8c61d78
Fixing indention + removing unnecessary iterator variable
NetroScript Jul 22, 2018
cba563b
Improved timeout for additional changes to showcase config items.
NetroScript Jul 22, 2018
47e6d23
Rewrite from array to object.
NetroScript Jul 22, 2018
1b6364e
Added missing curly braces
NetroScript Jul 22, 2018
bc00349
Check for the maximum number of possible showcases
NetroScript Jul 22, 2018
1773948
fixed ownworkshop
NetroScript Jul 22, 2018
1073e70
Changed name to remainingshowcases + break the loop at that point
NetroScript Jul 22, 2018
0b08247
Removed editing Showcases in editProfile(), added ediShowcaseItem() i…
NetroScript Jul 24, 2018
5e31a54
Removed obsolent variables, changed editShowcaseItem to callback only
NetroScript Jul 25, 2018
0356f54
Removed leftover `this` from Promise, create new object instead of us…
NetroScript Jul 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
252 changes: 245 additions & 7 deletions components/profile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,51 @@ SteamCommunity.prototype.setupProfile = function(callback) {
});
};

SteamCommunity.prototype.editShowcaseItem = function(showcase, slot, item, callback) {
const self = this;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a need for that

//The possible options, with the maximum number of slots and the corresponding type
const allowedoptions = {
"trade": {
"maxslots": 6,
"type": 4
},
"items": {
"maxslots": 10,
"type": 3
},
"games": {
"maxslots": 4,
"type": 2
}
};

if(!allowedoptions.hasOwnProperty(showcase)){
const err = new Error("The submitted showcase type has no editable items.");
return callback ? callback(err) : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using callback here instead of throw is a little counterintuitive for me, but I don't think that's enough to justify change request 🙂.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return undefined; looks really weird to me. I'd just use an if-statement here.

}
if(slot < 1 || slot > allowedoptions[showcase]["maxslots"]){
const err = new Error("The submitted slot is outside of range. (Allowed range: 1-"+allowedoptions[showcase]["maxslots"]+")");
return callback ? callback(err) : undefined;
}
if(!(item.hasOwnProperty("appid") || item.hasOwnProperty("item_contextid") || item.hasOwnProperty("item_assetid"))){
const err = new Error("The submitted item is not valid.");
return callback ? callback(err) : undefined;
}
const requestdata = item;
requestdata["slot"] = slot - 1;
requestdata["customization_type"] = allowedoptions[showcase]["type"];
requestdata["sessionid"] = self.getSessionID();
Copy link
Contributor

@Aareksio Aareksio Jul 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Objects are passed by reference, not by value. Check this out (minimal reproduction of the code above):

const item = { id: 1 };

const requestData = item;
requestData .slot = 1;

console.log(item, requestData ); // { id: 1, slot: 1 }, { id: 1, slot: 1 }

Solution for this may be ES6 spread and rest

const item = { id: 1 };

const requestData = {
  ...item,
  slot: 1
};

console.log(item, requestData); // { id: 1 }, { id: 1, slot: 1 }

self._myProfile("ajaxsetshowcaseconfig", requestdata, function (err, response, body) {

if (err || response.statusCode != 200) {
err = err || new Error("HTTP error " + response.statusCode);
return callback ? callback(err) : undefined;
}
return callback ? callback(null) : undefined;

});
};

SteamCommunity.prototype.editProfile = function(settings, callback) {
var self = this;
this._myProfile("edit", null, function(err, response, body) {
Expand All @@ -54,16 +99,17 @@ SteamCommunity.prototype.editProfile = function(settings, callback) {
}

var values = {};
form.serializeArray().forEach(function(item) {
form.serializeArray().forEach(function (item) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array.forEach callback is supplied with item index on 2nd parameter

values[item.name] = item.value;
});

for(var i in settings) {
if(!settings.hasOwnProperty(i)) {
var remainingshowcases = $(".profile_showcase_selector").length;

for (var i in settings) {
if (!settings.hasOwnProperty(i)) {
continue;
}

switch(i) {
switch (i) {
case 'name':
values.personaName = settings[i];
break;
Expand Down Expand Up @@ -103,15 +149,207 @@ SteamCommunity.prototype.editProfile = function(settings, callback) {
break;

case 'primaryGroup':
if(typeof settings[i] === 'object' && settings[i].getSteamID64) {
if (typeof settings[i] === 'object' && settings[i].getSteamID64) {
values.primary_group_steamid = settings[i].getSteamID64();
} else {
values.primary_group_steamid = new SteamID(settings[i]).getSteamID64();
}

break;

// TODO: profile showcases

case 'showcases':

//When supplying a new showcases array, remove the old showcase (order)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitechars (new lines) please

for (var val in values) {
if (val.indexOf("[") !== -1) {
if (val.split("[")[0] == "profile_showcase")
delete values[val];
}
}

for (var type in settings[i]) {

if (remainingshowcases === 0) {
break;
}

remainingshowcases--;

switch (settings[i][type].showcase) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new line should be above, not below 😉

case 'infobox':
values["profile_showcase[8]"] = 8;

if (settings[i][type].hasOwnProperty("values")) {
if (settings[i][type]["values"].hasOwnProperty("title")) {
values["rgShowcaseConfig[8][0][title]"] = settings[i][type]["values"]["title"];
}
if (settings[i][type]["values"].hasOwnProperty("notes")) {
values["rgShowcaseConfig[8][0][notes]"] = settings[i][type]["values"]["notes"];
}
}

break;

case 'artwork':
values["profile_showcase[13]"] = 13;

if (settings[i][type].hasOwnProperty("values")) {
for (var n = 0; n < 4; n++) {
values["profile_showcase[13][" + n + "][publishedfileid]"] = settings[i][type]["values"][n] || "";
}
}
break;

case 'trade':
values["profile_showcase[4]"] = 4;

if (settings[i][type].hasOwnProperty("values")) {
if (settings[i][type]["values"].hasOwnProperty("notes")) {
values["rgShowcaseConfig[4][6][notes]"] = settings[i][type]["values"]["notes"];
}
}
break;

case 'items':
values["profile_showcase[3]"] = 3;
break;

case 'game':
values["profile_showcase[6]"] = 6;

if (settings[i][type].hasOwnProperty("values")) {
values["rgShowcaseConfig[6][0][appid]"] = settings[i][type]["values"];
}
break;

case 'badge':
values["profile_showcase[5]"] = 5;

if (settings[i][type].hasOwnProperty("values")) {

if (settings[i][type]["values"].hasOwnProperty("style")) {
var styles = ["rare", "selected", null, "recent", "random"];
values["profile_showcase_style_5"] = styles.indexOf(settings[i][type]["values"]["style"]);
}

if (settings[i][type]["values"].hasOwnProperty("badges")) {
var types = ["badgeid", "appid", "border_color"];
for (var n = 0; n < 6; n++) {
for (var t in types) {
if (settings[i][type]["values"]["badges"][n] != undefined) {
if (settings[i][type]["values"]["badges"][n].hasOwnProperty(types[t])) {
values["rgShowcaseConfig[5][" + n + "][" + types[t] + "]"] = settings[i][type]["values"]["badges"][n][types[t]] || values["rgShowcaseConfig[5][" + n + "][" + types[t] + "]"] || "";
}
}
}
}
}
}

break;

case 'rareachievements':
values["profile_showcase[1]"] = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point I start to wonder what's the purpose of index in profile_showcase... Is this the order on page? If so, shouldn't it be ordered as user wanted? What if the user is lvl 10 and has only one showcase slot?

(All that is invalid if the index doesn't have anything to do with display order)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why I used the Array first, until I did some further testing and found out that the index of that doesn't matter, it has no influence at all it seems. The only important thing is the order of the parameters in the request, and because I remove all profile_showcase[] if the showcases are changed and add them again to the JSON object and JSON is sorted by timestamp, it means the showcases still have the same order on the profile as in the array. If the user is level 10 and has 1 showcase slot and submits 2 showcases in the array he will get an error and the whole EditProfile Call is invalid (but changes to f.e. the items in an item showcase are still valid).

break;

case 'screenshot':
values["profile_showcase[7]"] = 7;

if (settings[i][type].hasOwnProperty("values")) {
for (var n = 0; n < 4; n++) {
if (settings[i][type]["values"][n] != undefined) {
values["rgShowcaseConfig[7][" + n + "][publishedfileid]"] = settings[i][type]["values"][n];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual indexing here confuses me everytime.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? n<4 instead of f.e. for(number in settings[i][type]["values"]) - I did this because there are 4 items which you can change. So the user can't supply too many now, and if he defines f.e. only 2, the other 2 are left unchanged.

Copy link
Contributor

@Aareksio Aareksio Jul 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may do something like this:

settings[i][type].values.forEach((game, index) => {
  if (typeof game === 'undefined') return;
  values[`rgShowcaseConfig[7][${index}][publishfield]`] = game;
}

There are still two things someone may get confused on, settings[i][type] and `rgShowcaseConfig[7][${index}][publishfield]`, but it seems to be at least slightly better.

Classic for loops are good when our focus is on index, not value.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what if the user supplies 5 values. Nothing might happen, something might happen, who knows. Also all the code I saw was without using template strings, let, const, arrow functions, ... so I refrained from using these.

Copy link
Contributor

@andrewda andrewda Jul 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say template strings are fair game, because this library only officially supports Node.js 4+

node-steamcommunity/package.json

Line 34 in b7a3e51

"node": ">=4.0.0"
which also seems a little generous and might be able to be bumped, considering 4.0 is well past its LTS.

@DoctorMcKay, you added your stats reporting library to at least node-steam-tradeoffer-manager – what are the percentages of people using various different versions of Node, assuming you record those numbers?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NetroScript The very first word of profile.js is const. McKay have updated his libs to use templates and arrow functions (ref), seems like steamcommunity didn't get enough care though, that said, the example was about using Array.forEach instead of random loop.

So what if the user supplies 5 values. Nothing might happen, something might happen, who knows.

Well, the code is meant to be deterministic by nature, when user supplies 5 values, it will send one parameter more - what happens on Steam side is unknown indeed (it'll likely get discarded), if we know there's a restriction of 4 entries - we should somehow prevent the user from supplying more (eg. by throwing an error), instead of coercing the input.

You probably only want to use non descriptive loops when taking some sort of action multiple times, eg. sending n HTTP requests a few lines below, otherwise it leads to confusion - "Why 4? What's settings[i][type]["values"][n]?"

}
}
}
break;

case 'group':
values["profile_showcase[9]"] = 9;

if (settings[i][type].hasOwnProperty("values")) {
if (typeof settings[i][type]["values"] === 'object' && settings[i][type]["values"].getSteamID64) {
values["rgShowcaseConfig[9][0][accountid]"] = settings[i][type]["values"].getSteamID64();
} else {
values["rgShowcaseConfig[9][0][accountid]"] = new SteamID(settings[i][type]["values"]).getSteamID64();
}
}
break;

case 'review':
values["profile_showcase[10]"] = 10;

if (settings[i][type].hasOwnProperty("values")) {
values["rgShowcaseConfig[10][0][appid]"] = settings[i][type]["values"];
}
break;

case 'workshop':
values["profile_showcase[11]"] = 11;

if (settings[i][type].hasOwnProperty("values")) {
values["rgShowcaseConfig[11][0][appid]"] = settings[i][type]["values"]["appid"];
values["rgShowcaseConfig[11][0][publishedfileid]"] = settings[i][type]["values"]["publishedfileid"];
}
break;

case 'guide':
values["profile_showcase[15]"] = 15;

if (settings[i][type].hasOwnProperty("values")) {
values["rgShowcaseConfig[15][0][appid]"] = settings[i][type]["values"]["appid"];
values["rgShowcaseConfig[15][0][publishedfileid]"] = settings[i][type]["values"]["publishedfileid"];
}
break;

case 'achievements':
values["profile_showcase[17]"] = 17;

if (settings[i][type].hasOwnProperty("values") && settings[i][type]["values"].hasOwnProperty("achievements")) {
for (var n = 0; n < 7; n++) {
if (settings[i][type]["values"]["achievements"][n] != undefined) {
values["rgShowcaseConfig[17][" + n + "][appid]"] = settings[i][type]["values"]["achievements"][n]["appid"];
values["rgShowcaseConfig[17][" + n + "][title]"] = settings[i][type]["values"]["achievements"][n]["title"];
}
}
}
break;

case 'games':
values["profile_showcase[2]"] = 2;
break;

case 'ownguides':
values["profile_showcase[16]"] = 16;

if (settings[i][type].hasOwnProperty("values")) {
for (var n = 0; n < 4; n++) {
if (settings[i][type]["values"][n] != undefined) {
values["rgShowcaseConfig[16][" + n + "][appid]"] = settings[i][type]["values"][n]["appid"];
values["rgShowcaseConfig[16][" + n + "][publishedfileid]"] = settings[i][type]["values"][n]["publishedfileid"];
}
}
}
break;

case 'ownworkshop':
values["profile_showcase[12]"] = 12;

if (settings[i][type].hasOwnProperty("values")) {
for (var n = 0; n < 5; n++) {
if (settings[i][type]["values"][n] != undefined) {
values["rgShowcaseConfig[12][" + n + "][appid]"] = settings[i][type]["values"][n]["appid"];
values["rgShowcaseConfig[12][" + n + "][publishedfileid]"] = settings[i][type]["values"][n]["publishedfileid"];
}
}
}
break;
}
}
break;

}
}

Expand Down