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

List Controller #57

Merged
merged 7 commits into from Jan 4, 2020
Merged

List Controller #57

merged 7 commits into from Jan 4, 2020

Conversation

@DetectiveHuman
Copy link
Contributor

DetectiveHuman commented Dec 31, 2019

Based off issue #7

@DetectiveHuman DetectiveHuman requested a review from MattIPv4 Dec 31, 2019
list: data.id,
value: true
});
await this.db('feature_map').where({ list: data.id }).del();

This comment has been minimized.

Copy link
@MattIPv4

MattIPv4 Dec 31, 2019

Member

Do we want to refactor this to use update whilst we're here?

value: true
});
}
const features = Object.entries(data).filter((f) => f[0].substring(0, 8) === 'feature_').map((f) => {

This comment has been minimized.

Copy link
@MattIPv4

MattIPv4 Jan 3, 2020

Member
Suggested change
const features = Object.entries(data).filter((f) => f[0].substring(0, 8) === 'feature_').map((f) => {
const features = Object.entries(data).filter((f) => f[0].startsWith('feature_')).map((f) => {
addedFeatures.push(Number(key));
}
for (const oldFeature of oldFeatures) {
const feature = features.find((f) => f.find((f) => Number(f[0]) === oldFeature.feature));

This comment has been minimized.

Copy link
@MattIPv4

MattIPv4 Jan 3, 2020

Member

This hurts my head, what's happening here, why is f declared twice?

}
}
for (let [key, value] of Object.entries(data)) {
if (key.substring(0, 8) === 'feature_') {

This comment has been minimized.

Copy link
@MattIPv4

MattIPv4 Jan 3, 2020

Member
Suggested change
if (key.substring(0, 8) === 'feature_') {
if (key.startsWith('feature_')) {
DetectiveHuman and others added 2 commits Jan 3, 2020
Copy link
Member

MattIPv4 left a comment

Lgtm.

@MattIPv4 MattIPv4 merged commit 118dad0 into node Jan 4, 2020
2 checks passed
2 checks passed
build
Details
WIP Ready for review
Details
@MattIPv4 MattIPv4 deleted the list-controller branch Jan 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.