Skip to content
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

fix: checks on session and role-invite api and owner role #6131

Merged
merged 3 commits into from Jul 3, 2019
Merged

fix: checks on session and role-invite api and owner role #6131

merged 3 commits into from Jul 3, 2019

Conversation

shreyanshdwivedi
Copy link
Member

@shreyanshdwivedi shreyanshdwivedi commented Jul 1, 2019 •
edited

Fixes #6125

Short description of what this resolves:

There were bugs -

  • role deletion (from event overview page)
  • session isLocked updation.
  • missing owner from admin_statistics_api
  • missing permissions for owner

Changes proposed in this pull request:

  • Fixed bugs

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.

@auto-label auto-label bot added the fix label Jul 1, 2019
houndci-bot
houndci-bot reviewed Jul 1, 2019
View reviewed changes
app/api/role_invites.py Outdated Show resolved Hide resolved
houndci-bot
houndci-bot reviewed Jul 1, 2019
View reviewed changes
app/api/role_invites.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 1, 2019 •
edited

Codecov Report

Merging #6131 into development will decrease coverage by 0.01%.
The diff coverage is 60%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #6131      +/-   ##
===============================================
- Coverage         66.4%   66.38%   -0.02%     
===============================================
  Files              286      286              
  Lines            14359    14366       +7     
===============================================
+ Hits              9535     9537       +2     
- Misses            4824     4829       +5
Impacted Files Coverage Δ
app/api/role_invites.py 47.47% <0%> (ø) ⬆️
app/api/sessions.py 41.66% <0%> (ø) ⬆️
populate_db.py 79.75% <100%> (+0.34%) ⬆️
app/api/admin_statistics_api/users.py 76.78% <66.66%> (-0.58%) ⬇️
app/api/helpers/scheduled_jobs.py 22.22% <0%> (-4.05%) ⬇️

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 f5428b8...84fd6f4. Read the comment docs.

@shreyanshdwivedi
Copy link
Member Author

@kushthedude @mrsaicharan1 @uds5501 please review

@shreyanshdwivedi shreyanshdwivedi mentioned this pull request Jul 3, 2019
Merged
5 tasks
@shreyanshdwivedi shreyanshdwivedi changed the title fix: checks on session and role-invite api fix: checks on session and role-invite api and owner role Jul 3, 2019
uds5501
uds5501 reviewed Jul 3, 2019
View reviewed changes
populate_db.py Show resolved Hide resolved
@shreyanshdwivedi
Copy link
Member Author

@uds5501 @kushthedude @mrsaicharan1 please review :)

iamareebjamal
iamareebjamal reviewed Jul 3, 2019
View reviewed changes
migrations/versions/43e8c59337ag_.py Outdated


def upgrade():
op.execute("INSERT INTO permissions(role_id, service_id, can_create, can_read, can_update, can_delete) VALUES((SELECT id FROM roles WHERE name='owner'), 1, true, true, true, true)")
Copy link
Member

Choose a reason for hiding this comment

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

Why hardcoding service_id

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 fixed it. Please have a look

@fossasia fossasia deleted a comment Jul 3, 2019
@iamareebjamal iamareebjamal merged commit e71e64c into fossasia:development Jul 3, 2019
@shreyanshdwivedi shreyanshdwivedi deleted the sessionAndInvite branch July 4, 2019 06:16
iamareebjamal pushed a commit to iamareebjamal/open-event-server that referenced this pull request Aug 2, 2019
@shreyanshdwivedi @iamareebjamal
fix: checks on session and role-invite api and owner role (fossasia#6131
8058d2a 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@uds5501 uds5501 uds5501 left review comments

@iamareebjamal iamareebjamal iamareebjamal approved these changes

@kushthedude kushthedude kushthedude approved these changes

Assignees
No one assigned
Labels
fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Buggy checks on session and role-invite and owner role
5 participants
@shreyanshdwivedi @iamareebjamal @houndci-bot @uds5501 @kushthedude