Skip to content

Conversation

shreyanshdwivedi
Copy link
Member

Fixes #3354

Short description of what this resolves:

Currently, Event Role of every kind is able to see the following settings which should not be the case

Changes proposed in this pull request:

  • show Settings as an option only to admin or owner
  • if a non-admin and non-owner user tries to access Settings, redirect them to event.view

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter 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)

mrsaicharan1
mrsaicharan1 previously approved these changes Aug 18, 2019
Copy link
Member

@mrsaicharan1 mrsaicharan1 left a comment

Choose a reason for hiding this comment

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

LGTM.

uds5501
uds5501 previously approved these changes Aug 18, 2019
Copy link
Contributor

@uds5501 uds5501 left a comment

Choose a reason for hiding this comment

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

Logic looks LGTM

Anupam-dagar
Anupam-dagar previously approved these changes Aug 18, 2019
kushthedude
kushthedude previously approved these changes Aug 20, 2019
@shreyanshdwivedi
Copy link
Member Author

@CosmicCoder96 @mrsaicharan1 please review

@fossasia fossasia deleted a comment Aug 21, 2019
@fossasia fossasia deleted a comment Aug 23, 2019
@shreyanshdwivedi
Copy link
Member Author

@CosmicCoder96 please reveiew

@shreyanshdwivedi
Copy link
Member Author

@CosmicCoder96 please review this.

@fossasia fossasia deleted a comment Aug 27, 2019
return this.l10n.t('Settings');
},
beforeModel() {
let { currentUser } = this.authManager;
Copy link
Contributor

@abhinavk96 abhinavk96 Aug 27, 2019

Choose a reason for hiding this comment

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

how can this.authManager be currentUser?

Copy link
Member

@iamareebjamal iamareebjamal Aug 27, 2019

Choose a reason for hiding this comment

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

This is equivalent to let currentUser = this.authManager.currentUser

@mrsaicharan1 mrsaicharan1 self-requested a review August 27, 2019 11:51
@abhinavk96 abhinavk96 merged commit fffa333 into fossasia:development Aug 27, 2019
@shreyanshdwivedi shreyanshdwivedi deleted the ownerSettings branch August 27, 2019 13:11
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.

Event Dashboard: Show Settings only to Event Owners

7 participants