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.

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

Aip 6 rule #272

Merged
merged 3 commits into from
May 6, 2020
Merged

Aip 6 rule #272

merged 3 commits into from
May 6, 2020

Conversation

samparsky
Copy link
Contributor

Fixes issue #268

@Ivshti
Copy link
Member

Ivshti commented May 6, 2020

@samparsky well done, I found no issues!

Soon I'll make issues for adapting this for the new targeting rules system, which will just mean we'll have to rename most of the code in this PR. The matching of rules itself (AIP6) will have to be rewritten.

@Ivshti Ivshti merged commit 3443d84 into AmbireTech:master May 6, 2020
@@ -64,6 +72,14 @@ function makeRecorder(channelId) {
return hasAccess
}

const priceRuleModifyEvs = events.filter(x => x.type === eventTypes.update_price_rules)
Copy link
Member

Choose a reason for hiding this comment

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

you can use find here rather than relying on filter and then getting the last element

Copy link
Contributor Author

@samparsky samparsky May 6, 2020

Choose a reason for hiding this comment

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

Find will return the first item in the list it finds and not the last item

const priceRuleModifyEvs = events.filter(x => x.type === eventTypes.update_price_rules)
if (priceRuleModifyEvs.length) {
// if there are multiple evs only apply the latest
await updateChannelPriceMultiplicationRules(priceRuleModifyEvs[priceRuleModifyEvs.length - 1])
Copy link
Member

Choose a reason for hiding this comment

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

I think it's much better to not use a lambda function here but to call await channelsCol.updateOne( directly..it would be much more readable

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