-
Notifications
You must be signed in to change notification settings - Fork 5
selling cards functionality #116
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #116 +/- ##
============================================
- Coverage 23.58% 21.95% -1.63%
- Complexity 133 147 +14
============================================
Files 91 93 +2
Lines 1463 1635 +172
Branches 188 208 +20
============================================
+ Hits 345 359 +14
- Misses 1042 1197 +155
- Partials 76 79 +3
Continue to review full report at Codecov.
|
|
||
export function sellCards(cardIds) { | ||
console.log('cardss'); | ||
console.log(cardIds); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should remove console.log statements
//add selling car to listing | ||
$user = auth()->user(); | ||
$data = json_decode(Request::getContent(), true); | ||
foreach ($data as $cardId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not assume that the user passes a valid set of card IDs.
We need to verify that:
- The card exists
- The card is available for auction (not already on auction, not in battle, etc)
- The card is owned by the user trying to sell the card (thus the
$listing->user_id should not be set to the $user->id
$listing->price = $this->estimateValue($cardId); | ||
$listing->save(); | ||
|
||
//remove selling card from user's cards |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The card should not be removed from the user until the Listing has found a buyer, at which point the card will be removed from the old user and added to the new user instantaneously.
@@ -27,13 +27,15 @@ | |||
|
|||
Route::get('/battles', 'BattlegroundController@getAllBattles'); | |||
|
|||
Route::put('/sell', 'MarketplaceController@sellCards'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does sell take an array of cardIDs and not just a single cardID parameter to sell (like all other function calls)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because in the frontend the user selects one or more cards to sell so the cards are passed to the backend together. I think battles does it the same way in the contract controller
-Cards to be sold are added back to marketplace with the new price and removed from user's list of cards