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

Added the states column in the participants view #92

Merged
merged 1 commit into from Aug 14, 2017

Conversation

nikhilgupta1211
Copy link
Contributor

@nikhilgupta1211 nikhilgupta1211 commented Aug 9, 2017

A person can only have a single active request for an event. The State column shows the state of the active request of a user.

@nikhilgupta1211
Copy link
Contributor Author

image

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 92.488% when pulling e7fc87a on nikhilgupta1211:participatnsstate into fc76ae1 on openSUSE:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 92.488% when pulling e7fc87a on nikhilgupta1211:participatnsstate into fc76ae1 on openSUSE:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 92.672% when pulling e756d37 on nikhilgupta1211:participatnsstate into fc76ae1 on openSUSE:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 92.672% when pulling 587dec5 on nikhilgupta1211:participatnsstate into fc76ae1 on openSUSE:master.

@@ -16,4 +16,19 @@ def users_for_event(state)
requests = requests.where(state: state) if state != 'all'
requests.distinct.pluck('users.email')
end

def state_label(state)
Copy link
Member

Choose a reason for hiding this comment

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

Just return the css class and do the rest in the view because it is always the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@@ -10,7 +10,7 @@ def participants
{ label: @event.name, url: event_path(@event) },
{ label: 'participants' }
]
@requests = @event.travel_sponsorships.includes(:user).select(:user_id).distinct.accessible_by(current_ability)
@requests = @event.travel_sponsorships.includes(:user).group(:user_id).accessible_by(current_ability)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the previous query we are only selecting the user_id from a request for distinct requests if we also select state here then the requests will not be distinct, also group returns the latest request of the user(active request) I don't know if distinct also do the same.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 92.672% when pulling 2e76ce8 on nikhilgupta1211:participatnsstate into fc76ae1 on openSUSE:master.

def state_label(state)
case state
when 'submitted'
'label label-primary'
Copy link
Member

Choose a reason for hiding this comment

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

label could also go to the view

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes ok :)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 92.704% when pulling 2f2fc0c on nikhilgupta1211:participatnsstate into bc97987 on openSUSE:master.

@ChrisBr ChrisBr merged commit 49da543 into openSUSE:master Aug 14, 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

3 participants