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

Booth Assigment management #2087

Merged
merged 12 commits into from
Oct 20, 2017
Merged

Booth Assigment management #2087

merged 12 commits into from
Oct 20, 2017

Conversation

bertocq
Copy link
Collaborator

@bertocq bertocq commented Oct 19, 2017

Where

Related Issues: #2034

What

We need to separate the listing of Booths assigned to a Poll from the Booth Assigment manage page:

  • On the Poll details we'll just list Booths assigned to it with a quick access button to Booth Assigment for that Poll. Without "Add booth" or "Remove booth" buttons

  • We need a new Admin > Polls > Booth Assignments menu that:
    A - List all current or incoming Polls
    B - Selection a Poll we can see all Booths with an "Assign" or "Unassign" ajax button (depending on the existance of a booth assigment for that booth & poll) that changes on click without full page render

How

  • Creating two new routes:

    • /admin/polls/booth_assignments for list A
    • /admin/polls/:id/booth_assignments/manage for list B
  • Creating views for each of them (copy&pasting code instead of trying to be smart on current existing views and having action_name and controller_name conditions all around

  • Switching Admin::Poll::BoothAssignmentsController create and destroy methods to work with the ajax buttons to Assign & Unassign booths (creating/destroying a Booth Assigment object)

  • Creating a Poll::Booth method assignment_on_poll(poll) that returns the existing (or not) booth assignment between that Booth and the poll (check Warning section to understand decision before rage kicks in)

Screenshots

No more screenshots, GIFs!
mecomaundemonio

Tests

Switched from old to new way to manage booth assigments, plus added an scenario to correctly test new Booth Assigment menu poll & booth listing

Deployment

As usual

Warnings

Listing all booths and making a query for each an every one to check the existance of a booth-assigment with the curren poll is not nice with performance if there are a lot of Booths, heck its not even about performance I felt like 💩 doing it but... we agreed upon this solution to get this working ASAP

For me best solution would be to:

  1. Get a list of booths currently with a booth assigment with the poll assigned_booths
  2. Get the list of alll booths and substract the assigned booths list resulting in unassigned_booths
  3. Merging both list (having a boolean flag to distinguish assigned/unassinged) and using that list instead of crappy @booths = ::Poll::Booth.all at Admin::Poll::BoothAssignmentsController#manage

PR's welcome to fix it if you have read this far!!

@bertocq bertocq changed the title [WIP] Booth Assigment management Booth Assigment management Oct 19, 2017
Copy link
Contributor

@MariaCheca MariaCheca left a comment

Choose a reason for hiding this comment

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

👍

@@ -1,10 +1,16 @@
<%= render "/admin/poll/polls/poll_header" %>


Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove this extra space. Let the code make some friends! 😁

@bertocq bertocq merged commit 8e649bc into master Oct 20, 2017
@bertocq bertocq deleted the poll_booth_management branch October 20, 2017 20:08
@bertocq bertocq mentioned this pull request Oct 26, 2017
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

2 participants