Skip to content

Conversation

@AVVS
Copy link
Member

@AVVS AVVS commented Oct 30, 2019

No description provided.


const { USERS_METADATA, USERS_AUDIENCE } = require('../../constants');

class User {
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
class User {
class UserMetadata {

return redis[name](keys.length, keys, argv);
});

return Promise.all(scripts).then((res) => UpdateMetadata.mapScriptResponse($scriptKeys, res));
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
return Promise.all(scripts).then((res) => UpdateMetadata.mapScriptResponse($scriptKeys, res));
const res = await Promise.all(scripts);
return UpdateMetadata.mapScriptResponse($scriptKeys, res);

const operations = metaOps.map((meta, idx) => UpdateMetadata.handleAudience(pipe, keys[idx], meta));
return pipe.exec()
.then(handlePipeline)
.then((res) => UpdateMetadata.mapMetaResponse(operations, res));
Copy link
Member Author

Choose a reason for hiding this comment

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

тут тоже

const res = handlePipeline(await pipe.exec());
return UpdateMetadata.mapMetaResponse(operations, res);

*/
async batchUpdate(opts) {
const { redis } = this;
assert(!(redis instanceof Pipeline), 'impossible to use with pipeline');
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
assert(!(redis instanceof Pipeline), 'impossible to use with pipeline');
assert(!(redis instanceof Pipeline), 'impossible to use without pipeline');

audienceWork.push(
redis.sadd(this.getAudienceKey(id), audience)
);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

sadd принимает массив значений, можно упростить:

redis.sadd(this.getAudienceKey(id), audiences)

потому что:

// Arguments to commands are flattened, so the following are the same:
redis.sadd("set", 1, 3, 5, 7);
redis.sadd("set", [1, 3, 5, 7]);

Copy link
Member Author

Choose a reason for hiding this comment

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

по сути весь метод .batchAdd не нужен тк он равнозначен .add

* @param audience
* @returns {Promise<void>}
*/
update(id, hashKey, value, audience) {
Copy link
Member Author

Choose a reason for hiding this comment

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

я бы добавил проверку наличия всех аргументов, иначе может программист допустить ошибку и забыть добавить какой-то аргумент

assert там использовать или что-то в этом роде

@pajgo pajgo force-pushed the feat/delinactive/update-metadata-rework branch from a8bc8e0 to 0b092f6 Compare October 31, 2019 08:09
const metaOps = Array.isArray(metadata) ? metadata : [metadata];

if (metaOps.length !== audiences.length) {
return Promise.reject(new HttpStatusError(400, 'audiences must match metadata entries'));
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
return Promise.reject(new HttpStatusError(400, 'audiences must match metadata entries'));
throw new HttpStatusError(400, 'audiences must match metadata entries');

@pajgo
Copy link
Collaborator

pajgo commented Jan 9, 2020

#436 Dup

@pajgo pajgo closed this Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants