Bugfix/remove reward metric key#2767
Conversation
…reward-metric-key
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 39 changed files in this pull request and generated 9 comments.
Files not reviewed (2)
- clientlibs/js/package-lock.json: Language not supported
- types/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const result = await service.sendReward(mockUser, request, mockLogger as any); | ||
|
|
||
| expect(result.message).toBe('Reward sent to mooclet successfuly.'); |
There was a problem hiding this comment.
Typo in test assertion: "successfuly" should be "successfully".
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| const whereClauseParams = { | ||
| enrolling: 'enrolling', | ||
| assign: 'assign', | ||
| context, | ||
| site, | ||
| target, | ||
| }; |
There was a problem hiding this comment.
The whereClauseParams object includes an assign: 'assign' parameter that is not used in the SQL query. This parameter appears in the params but is not referenced in the whereExperimentsClause string. This should be removed to avoid confusion.
Remove the unused parameter:
const whereClauseParams = {
enrolling: 'enrolling',
context,
site,
target,
};There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| async function doSendRewardByExperimentId(client: UpgradeClient) { | ||
| try { | ||
| const response = await client.sendReward({ | ||
| rewardValue, | ||
| experimentId, | ||
| }); | ||
| console.log('\n[Send Reward by ExperimentId response]:', JSON.stringify(response)); | ||
| } catch (error) { | ||
| logAxiosError('Send Reward by ExperimentId', error); | ||
| } | ||
| } | ||
|
|
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@danoswaltCL I've opened a new pull request, #2768, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@danoswaltCL I've opened a new pull request, #2769, to work on those changes. Once the pull request is ready, I'll request review from you. |
danoswaltCL
force-pushed
the
bugfix/remove-reward-metric-key
branch
from
f58b02e to
1b44e0d
Compare
#2741
/logbased reward system and all tie-ins to upgrade metrics systems/rewardendpoint to client controller, which will lookup experiment info and do a more direct call to moocletsendReward()endpoint to TS client lib