Skip to content

Conversation

@prateekj117
Copy link
Member

Fixes #5857

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.
  • The unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All the functions created/modified in this PR contain relevant docstrings.

Short description of what this resolves:

Delete a favourite event using Event ID.

@prateekj117
Copy link
Member Author

@uds5501 @mrsaicharan1 @shreyanshdwivedi Can you guys please have a look at this. I have tested everything locally, generated the docs, checked the response, everything works fine. Can you guys please see why the tests are failing.

@mrsaicharan1
Copy link
Member

Mostly the docs which have been updated in the .apib fails. Just the check the request once more maybe?

@prateekj117
Copy link
Member Author

@mrsaicharan1 Yaa, dredd is failing, that's sure. I would try one more thing.

## Favourite Events Detail [/v1/user-favourite-events/{event_id}]
+ Parameters
+ user_favourite_event_id: 1 (integer) - ID of the Favourite Event
+ event_id: 1 (integer) - ID of the Event in the form of an integer
Copy link
Member

Choose a reason for hiding this comment

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

Changing name of parameters won't fix anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

@iamareebjamal But it should be changed, right?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, your version is more explicit, but if you changed it in hopes that it'll fix the error, then it won't

@prateekj117
Copy link
Member Author

@uds5501 @mrsaicharan1 @shreyanshdwivedi While using current_user, it shows current_user is None. Any idea why?

@shreyanshdwivedi
Copy link
Member

@prateekj117 are you trying FE or Postman? If postman maybe you haven't authenticated yourself

@prateekj117
Copy link
Member Author

@shreyanshdwivedi Trying postman. I have authenticated myself.

@fossasia fossasia deleted a comment Jun 20, 2019
@fossasia fossasia deleted a comment Jun 20, 2019
@fossasia fossasia deleted a comment Jun 21, 2019
@fossasia fossasia deleted a comment Jun 21, 2019
@fossasia fossasia deleted a comment Jun 21, 2019
"""
User Favourite Events detail by id
"""
@jwt_required()
Copy link
Member

Choose a reason for hiding this comment

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

paranthesis is redundant

Copy link
Member Author

Choose a reason for hiding this comment

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

@iamareebjamal It shows error when not using paranthesis.
Screenshot from 2019-06-22 14-20-29

Copy link
Member

Choose a reason for hiding this comment

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

OK

@fossasia fossasia deleted a comment Jun 21, 2019
@codecov
Copy link

codecov bot commented Jun 21, 2019

Codecov Report

Merging #6063 into development will decrease coverage by 0.02%.
The diff coverage is 46.66%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #6063      +/-   ##
===============================================
- Coverage         66.4%   66.37%   -0.03%     
===============================================
  Files              286      286              
  Lines            14348    14359      +11     
===============================================
+ Hits              9528     9531       +3     
- Misses            4820     4828       +8
Impacted Files Coverage Δ
app/api/user_favourite_events.py 60% <46.66%> (-8.19%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41c1deb...5e370a2. Read the comment docs.

view_kwargs['id'] = None

@jwt_required()
def before_delete_object(self, view_kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

This is unneeded. Remove

user_favourite_event = UserFavouriteEvent.query.filter_by(
user=current_user, event_id=view_kwargs['id']).first()
except NoResultFound:
raise ObjectNotFound({'source': ''}, "Object: not found")
Copy link
Member

Choose a reason for hiding this comment

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

Set proper source

@prateekj117
Copy link
Member Author

@iamareebjamal Please review.

@fossasia fossasia deleted a comment Jun 22, 2019
@iamareebjamal
Copy link
Member

Follow semantic PR title

@prateekj117 prateekj117 changed the title Get and delete event favourite using event_id fix: Get and delete event favourite using event_id Jun 22, 2019
@auto-label auto-label bot added the fix label Jun 22, 2019
@prateekj117
Copy link
Member Author

@iamareebjamal Please review.

@fossasia fossasia deleted a comment Jul 2, 2019
@iamareebjamal
Copy link
Member

I thought this was already merged

@iamareebjamal iamareebjamal merged commit 94489bb into fossasia:development Jul 2, 2019
iamareebjamal pushed a commit to iamareebjamal/open-event-server that referenced this pull request Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delete User Favorite By EventId

5 participants