Skip to content
This repository was archived by the owner on Oct 22, 2021. It is now read-only.

WIP: Implement scope#31

Open
cmfcmf wants to merge 6 commits intomasterfrom
scope
Open

WIP: Implement scope#31
cmfcmf wants to merge 6 commits intomasterfrom
scope

Conversation

@cmfcmf
Copy link
Copy Markdown
Contributor

@cmfcmf cmfcmf commented Jun 20, 2018

closes #20

cmfcmf and others added 2 commits June 20, 2018 14:54
Co-Authored-By: Frederike Ramin <frederike.ramin@student.hpi.de>
Co-Authored-By: Frederike Ramin <frederike.ramin@student.hpi.de>
GoesOnTangents
GoesOnTangents previously approved these changes Jun 20, 2018
Copy link
Copy Markdown
Contributor

@GoesOnTangents GoesOnTangents left a comment

Choose a reason for hiding this comment

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

Could have been done more gracefully but good enough for now...

Comment thread src/rule-parser.js Outdated
});
const amountSoFar = awardedSoFar.length === 0 ? 0 : awardedSoFar[0].amount;

const event = feathersContext.data; // user_id: blah, context: context: course_id: 42
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove comment, please

Comment thread src/hooks/achievement-rule-checker.js Outdated
return true;
});
if (!hasValidFieldNames) {
continue;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe throw an error here?

}
});

assert.equal(achievements.length, 2);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actually, this is wrong. The scope is empty in the yaml, but we just assume user_id for it. Actually here, only User 1 should be granted the GlobalAchievement. What we tested for here, is scope: [user_id] or an empty scope field.

@cmfcmf cmfcmf added the scope label Aug 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scope

4 participants