Skip to content

fix: Dasherize admin sales APIs and fix admin/sales/location API #5528

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 21, 2019
Merged

fix: Dasherize admin sales APIs and fix admin/sales/location API #5528

merged 2 commits into from
Jan 21, 2019

Conversation

@YogeshSharma0201
Copy link
Contributor

@YogeshSharma0201 YogeshSharma0201 commented Jan 21, 2019

Fixes #5527

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:

  • Admin sales apis are dasherized.
  • location_name is used as id, earlier it was not returning any id, which lead to errors on frontend. (Since location_name is unique i have used that as id, please do suggest any better alternative approch).
locations = self.session.query(
           Event.location_name,
           Event.location_name.label('id')) \
               .group_by(Event.location_name) \
               .filter(Event.location_name != None) \
               .cte()

houndci-bot
houndci-bot reviewed Jan 21, 2019
View reviewed changes
@YogeshSharma0201 YogeshSharma0201 mentioned this pull request Jan 21, 2019
Merged
5 tasks
abhinavk96
abhinavk96 suggested changes Jan 21, 2019
View reviewed changes
@YogeshSharma0201
Copy link
Contributor Author

YogeshSharma0201 commented Jan 21, 2019

@CosmicCoder96 is not None doesn't work i have tried that.
https://stackoverflow.com/questions/21784851/sqlalchemy-is-not-null-select#comment57501387_21784915

@codecov
Copy link

codecov bot commented Jan 21, 2019 •
edited
Loading

Codecov Report

Merging #5528 into development will increase coverage by 0.01%.
The diff coverage is 85.71%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #5528      +/-   ##
===============================================
+ Coverage        63.11%   63.13%   +0.01%     
===============================================
  Files              265      265              
  Lines            12720    12726       +6     
===============================================
+ Hits              8028     8034       +6     
  Misses            4692     4692
Impacted Files Coverage Δ
app/api/admin_sales/utils.py 40% <ø> (ø) ⬆️
app/api/admin_sales/events.py 92.85% <100%> (+0.54%) ⬆️
app/api/admin_sales/organizer.py 85.71% <100%> (+0.86%) ⬆️
app/api/admin_sales/locations.py 64.1% <66.66%> (+1.94%) ⬆️

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 32689d9...00cc8be. Read the comment docs.

@iamareebjamal
Copy link
Member

iamareebjamal commented Jan 21, 2019

Then use query.filter(User.name.isnot(None))

@YogeshSharma0201
Copy link
Contributor Author

YogeshSharma0201 commented Jan 21, 2019

@iamareebjamal i have changed that.

@iamareebjamal
Copy link
Member

iamareebjamal commented Jan 21, 2019

Please change the PR title to be more descriptive and according to the guidelines

@YogeshSharma0201 YogeshSharma0201 changed the title fix_admin_sales_api Dasherize admin sales apis and fix admin/sales/location api Jan 21, 2019
@iamareebjamal iamareebjamal changed the title Dasherize admin sales apis and fix admin/sales/location api fix: Dasherize admin sales APIs and fix admin/sales/location API Jan 21, 2019
@abhinavk96 abhinavk96 merged commit a3b646e into fossasia:development Jan 21, 2019
mrsaicharan1 pushed a commit to mrsaicharan1/open-event-server that referenced this pull request Jan 30, 2019
@prabhupant @mrsaicharan1
docs(api): Modified API blueprints of Tickets and ticket tags (fossas…
b0a8667 
…ia#5512)

Added package in local installation docs

Added test for authentication

Added docstring for auth test

Unused modules of auth test removed

Added package in local installation docs (fossasia#5515)

Changed module import style

test exception handled

User import

Whitespace commit

Export csv support for sessions

Add test database instructions

Update local.md

Added test for authentication (fossasia#5517)

* Added package in local installation docs

* Added test for authentication

* Added docstring for auth test

* Unused modules of auth test removed

* Changed module import style

* test exception handled

* User import

* Whitespace commit

Remove authorisation constraint for discount and access code list

Fix Admin Sales Api (fossasia#5528)

Add hooks for admin statistics (fossasia#5530)

removed unused packages

continuation line indent'

removed test doc commmit to separate PR

Removed redundant print statement

Add seed data for custom placeholders (fossasia#5533)

Disable registrat constraint to update attendees (fossasia#5544)

Frontend follows a flow where non registrar user has to edit an attendee while placing an order.

remove marketer and service fee constraints

Currently, the APIs are not functioning correctly with these intricate configurations.

docs(api): Remove unused line no 24100 in API blueprint (fossasia#5541)

Orders API: Remove verified user constraint (fossasia#5548)

Lax permissions to view discount codes (fossasia#5551)

Read from environment variables when creating the super admin user

Added test database docs for running tests locally (fossasia#5539)

* Added package in local installation docs

* Added test for authentication

* Added docstring for auth test

* Unused modules of auth test removed

* Changed module import style

* test exception handled

* User import

* Whitespace commit

* Add test database instructions

* Update local.md

* Export csv support for sessions

* removed unused packages

* continuation line indent'

* removed test doc commmit to separate PR

* Added test database docs

* Added instructions for test database docs

Removed stream response

removed unnecessary import

Add seed data for custom placeholders (fossasia#5533)

Disable registrat constraint to update attendees (fossasia#5544)

Frontend follows a flow where non registrar user has to edit an attendee while placing an order.

remove marketer and service fee constraints

Currently, the APIs are not functioning correctly with these intricate configurations.

docs(api): Remove unused line no 24100 in API blueprint (fossasia#5541)

Orders API: Remove verified user constraint (fossasia#5548)

Lax permissions to view discount codes (fossasia#5551)

Read from environment variables when creating the super admin user

Added test database docs

Added fields for csv export sessions
mrsaicharan1 pushed a commit to mrsaicharan1/open-event-server that referenced this pull request Jan 30, 2019
@YogeshSharma0201 @mrsaicharan1
Fix Admin Sales Api (fossasia#5528)
ffa2c21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iamareebjamal iamareebjamal iamareebjamal approved these changes

+2 more reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@abhinavk96 abhinavk96 abhinavk96 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dasherize admin sales apis and fix admin/sales/location api.

4 participants

@YogeshSharma0201 @iamareebjamal @houndci-bot @abhinavk96