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.

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

Removed officer alert in admin section #902

Merged
merged 3 commits into from
Oct 3, 2017
Merged

Conversation

MariaCheca
Copy link

@MariaCheca MariaCheca commented Oct 2, 2017

What

Removed alert appearing in admin section when an admin user also poll officer is in this section, as requested.

Made alert for poll officers just appear in the Officing section (http://localhost:3000/officing) instead all over admin section.

Test

As usual.

Deployment

As usual.

@bertocq
Copy link

bertocq commented Oct 2, 2017

But... should that notice keep appearing for Officers? If so, where? I feel like there's something missing here... either:

  • Totally remove the admin.officing_booth.title keys on both en and es admin.yml files
  • Just remove the notice for the admins, but not for the officers (I suspect that app/views/layouts/admin.html.erb is being used underneath on the officing panel, thus the confusion thinking that there was a second place that was appearing/being rendered)

Copy link

@bertocq bertocq left a comment

Choose a reason for hiding this comment

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

That's better! :) Most importantly... you've checked it works right? ^_^

@MariaCheca
Copy link
Author

@bertocq yep 🙃

Copy link
Member

@voodoorai2000 voodoorai2000 left a comment

Choose a reason for hiding this comment

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

👌

Removed quotation marks as the value of the controller class parent is not returned as a string.
@MariaCheca MariaCheca merged commit 489282b into master Oct 3, 2017
@MariaCheca MariaCheca deleted the booth_officer_alert_fix branch October 3, 2017 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants