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

Track request acceptance and activation #1597

Conversation

3 participants
@AEtherC0r3
Copy link
Contributor

AEtherC0r3 commented Jul 17, 2017

  • Add finite state machine to the track model and allow the state to be changed
  • Make the state column not null and add the default value 'new' to it. Also, the regular tracks are marked as confirmed by default
  • Create cfp for tracks and give to signed_in users the ability to create requests for tracks
  • Turn the cfp scopes into class methods, because, if the answer is nil then a scope will return all records.
  • Require a room and dates for accepted/confirmed self-organized tracks (This will be used for the scheduling later)
  • Allow the track submitter to request specific dates for his tracks
  • Don't allow the requests for tracks to be deleted
  • Don't allow the submitter and the track organizers to edit a request after it has been accepted
  • Make the cfp_active column not null and set the value to true if it is nill (the track is an old regular track)
  • Restrict track selection in proposals to only the confirmed tracks with the cfp_active flag
  • Make the tracks views similar to the proposals views
  • Add a new field to the tracks (relevance) so the submitter of a request can explain how it relates to the conference

Important workflow notes:

  • When a track request is accepted then the role of the track organizer for that track is created
  • When a track is confirmed then the submitter is assigned that role
  • When a track is cancelled/withdrawn then that role is revoked from all the users that have it and the track's association with events is removed
@AEtherC0r3

This comment has been minimized.

Copy link
Contributor Author

AEtherC0r3 commented Jul 17, 2017

When an organizer is about to accept a Track request, I need to somehow prompt him to fill in the room and dates for the track. Should I redirect him to #edit or use something like a popup?

end

def ready_to_accept
update_state(:readiness_to_accept, 'Track request is signaling readiness for acceptance!')

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 17, 2017

Contributor

Can't help myself thinking of
http://top40-charts.com/songs/lyrics.php?sid=28986&string=Catch%20Me%20When%20I%27m%20Falling

Perhaps in our case we are not signaling anything to anyone, but rather marking the track as a possible acceptance :)

@sayalilunkad

This comment has been minimized.

Copy link

sayalilunkad commented Jul 17, 2017

@AEtherC0r3 I would think a redirect would be more appropriate here.

event :restart do
transitions to: :new, from: [:rejected, :withdrawn, :canceled]
end
event :readiness_to_accept do

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 17, 2017

Contributor

to_accept like the state.

end

events.each do |event|
event.track = nil

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 17, 2017

Contributor

What if the event is already scheduled in a room?

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Jul 18, 2017

Author Contributor

Because the track organizer can't accept and schedule events yet, I'm only removing the track from the event. When the scheduling for tracks is implemented, I will also destroy the track's schedule and revert the event back to the new state

end

# Revokes the track organizer role and removes the track from events that have it set
def revoke_role_and_cleanup

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 17, 2017

Contributor

You should add some information about these kinds of new workflows in your PR's description.

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Aug 7, 2017

Author Contributor

I've updated the description

begin
send(transition)
rescue Transitions::InvalidTransition => e
error += "State update failed. #{e.message} "

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 17, 2017

Contributor

Perhaps the error message can be a bit more informative regarding which event and/or which state the function was trying to change.

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Jul 18, 2017

Author Contributor

The message of the exception already contains that info

restart_admin_conference_program_track_path(@conference.short_title, track),
method: :patch, id: "restart_track_#{track.id}"

- if track.transition_possible? :readiness_to_accept

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 17, 2017

Contributor

:to_accept

- if track.transition_possible? :readiness_to_accept
%li= link_to 'Signal readiness to accept track request',
ready_to_accept_admin_conference_program_track_path(@conference.short_title, track),
method: :patch, id: "ready_to_accept_track_#{track.id}"

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 17, 2017

Contributor

ready not needed

method: :patch, id: "restart_track_#{track.id}"

- if track.transition_possible? :readiness_to_accept
%li= link_to 'Signal readiness to accept track request',

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 17, 2017

Contributor

Possible acceptance perhaps?

@@ -15,6 +15,9 @@
%th Color

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 17, 2017

Contributor

I'd move this column closer to the title, so that the first columns are either the same or similar to the original view, ie
name, descr, color, room, dates, submitter, state, actions
I don't think we need to show short_name at all.

@AEtherC0r3 AEtherC0r3 force-pushed the AEtherC0r3:track_request_acceptance_and_activation branch from cebba9b to efb88e4 Jul 18, 2017


describe '.for_tracks' do
it 'returns the cfp for tracks when it exists' do
create(:cfp, cfp_type: 'tracks', program: conference.program, end_date: Date.today)

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 18, 2017

Contributor

assign this to a variable, and then check if this is the returned cfp


visit edit_admin_conference_program_cfp_path(conference.short_title, conference.program.cfp)
expect(current_path).to eq(edit_admin_conference_program_cfp_path(conference.short_title, conference.program.cfp))

# Event, booth, track cfps exist
cft = create(:cfp, cfp_type: 'tracks', program: conference.program)

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 18, 2017

Contributor

Let's give the variables a more self-explanatory name, ie call_for_tracks.

@@ -8,7 +8,7 @@

- if @program.tracks.any?
= f.input :track_id, as: :select,
collection: @program.tracks.map {|track| ["#{track.name}", track.id] },
collection: @program.tracks.map {|track| ["#{track.name}", track.id] if !track.self_organized? || track.confirmed? && track.cfp_active }.compact,

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 18, 2017

Contributor

Space after curly bracket please

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 18, 2017

Contributor

Shall we extract that to a method for our program to simply our code here?

@@ -53,6 +61,21 @@
- else
%i.fa.fa-check
%td
- if track.self_organized? && track.room

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 18, 2017

Contributor

Room is not mandatory for regular tracks as we know them till now, but can't we just be showing this? If and when an organizer might want to assign a room at a track, the organizer should be free to do that. Same with start/end dates. What do you think?

if @track.room && @track.start_date && @track.end_date
update_state(:accept, "Track request for #{@track.name} accepted!")
else
flash[:notice] = 'Please make sure that the track has a room and start/end dates before accepting it'

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 18, 2017

Contributor

This sounds like an error, not notice

@AEtherC0r3 AEtherC0r3 force-pushed the AEtherC0r3:track_request_acceptance_and_activation branch from 921c994 to a2066ee Jul 20, 2017

@differentreality
Copy link
Contributor

differentreality left a comment

admin/tracks#show should show both track events and track details (including track organizers).

end

def to_accept
update_state(:to_accept, "Track request for #{@track.name} marked as a possible acceptance!")

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

I think the 'request' part is redundant in the message. Track X marked as a possible acceptance / Track X was accepted / etc is enough.

@@ -4,7 +4,7 @@ class TracksController < ApplicationController
load_and_authorize_resource through: :program, find_by: :short_name

def index
@tracks = current_user.tracks.where(program: @program)
@tracks = @tracks.map{ |track| track if track.submitter == current_user }.compact

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

Why not just @tracks = @tracks.where(submitter: current_user) ?

@@ -39,9 +38,33 @@ def update
end
end

def restart
update_state(:restart, 'Track request re-submitted.')

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

Let's remove request, and include the track's name

track.new_record? && track.program.cfps.for_tracks.try(:open?)
end

can [:index, :show, :restart, :confirm, :withdraw], Track, submitter_id: user.id

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

I should be able to access show action of confirmed tracks, to get their details.

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Jul 25, 2017

Author Contributor

You can find the details of the track in admin/Tracks#show

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Jul 27, 2017

Author Contributor

The users can see the name and description of every confirmed track included in the cfp in the conference's splashpage

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 27, 2017

Contributor

Let's also show them then the room and the dates of the track, when available.

@@ -56,7 +56,7 @@ def dynamic_association(association_name, title, form_builder, options = {})
end

def tracks(conference)
all = conference.program.tracks.map {|t| t.name}
all = conference.program.tracks.where(state: 'confirmed', cfp_active: true).pluck(:name)

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

Let's add a scope for confirmed and cfp_active tracks.

- if track.start_date
= track.start_date.strftime('%A, %B %-d. %Y')
- else
N/A

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

We should remove this all together, and show nothing instead.


%p
%strong
Why do I need to add more information?

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

This is not needed, is it? We don't ask people to add extra information, except for the fields that they completed during request creation.


%p
%strong
Why do I need to register to the conference?

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

Same for this one.

%td
- if track.end_date
To:
= track.end_date.strftime('%A, %B %-d. %Y')

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

What about the room?

%span{ title: track.state.humanize, class: 'fa fa-ban'}
%td
= link_to(conference_program_track_path(@conference.short_title, track)) do
= track.name

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

What about adding a background with the track's color, instead of having a separate column for it?

- if track.end_date
To:
= track.end_date.strftime('%A, %B %-d. %Y')
%td.pull-right

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

Something looks broken on the table (the horizontal line gets interrupted)

screenshot from 2017-07-23 02-35-37

- if @conference.venue
= f.input :room, as: :select, collection: (@conference.venue.rooms).map {|room| ["#{room.name}", room.id]}, include_blank: true, label: 'Room', input_html: { class: 'select-help-toggle', required: @track.self_organized_and_accepted_or_confirmed? }
- else
Please add a venue with rooms, if you want to select a room for the track.
= f.input :description, input_html: {rows: 2, data: { provide: 'markdown-editable' } }, hint: markdown_hint

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

Let's add a hint that this will become public.

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Jul 25, 2017

Author Contributor

This is fixed in the non admin form

- if @conference.venue
= f.input :room, as: :select, collection: (@conference.venue.rooms).map {|room| ["#{room.name}", room.id]}, include_blank: true, label: 'Room', input_html: { class: 'select-help-toggle', required: @track.self_organized_and_accepted_or_confirmed? }
- else
Please add a venue with rooms, if you want to select a room for the track.
= f.input :description, input_html: {rows: 2, data: { provide: 'markdown-editable' } }, hint: markdown_hint

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 22, 2017

Contributor

And let's add one more field for submitter to explain how this track relates to the conference, why it should be accepted, and what's the users relation to the track's content.

@AEtherC0r3 AEtherC0r3 force-pushed the AEtherC0r3:track_request_acceptance_and_activation branch 2 times, most recently from a238f49 to df7d212 Jul 25, 2017

@AEtherC0r3 AEtherC0r3 changed the title [WIP] Track request acceptance and activation Track request acceptance and activation Jul 27, 2017

@AEtherC0r3 AEtherC0r3 force-pushed the AEtherC0r3:track_request_acceptance_and_activation branch from 31a0be2 to 07f148e Jul 27, 2017

%td
= link_to track.submitter.name, admin_user_path(track.submitter) if track.self_organized?

This comment has been minimized.

Copy link
@differentreality

differentreality Jul 27, 2017

Contributor

This potentially throws an exception

ActiveRecord::RecordNotFound in Admin::UsersController#show
Couldn't find Conference with 'id'=152

app/models/user.rb:168:in block in get_roles' app/models/user.rb:167:in get_roles'
app/views/admin/users/show.html.haml:25:in block in _app_views_admin_users_show_html_haml___579614431558334695_133750840' app/views/admin/users/show.html.haml:18:in each'
app/views/admin/users/show.html.haml:18:in `_app_views_admin_users_show_html_haml___579614431558334695_133750840'

@AEtherC0r3 AEtherC0r3 force-pushed the AEtherC0r3:track_request_acceptance_and_activation branch 2 times, most recently from 4eecbe8 to 71592a9 Jul 27, 2017

@AEtherC0r3 AEtherC0r3 removed the in progress label Jul 27, 2017

@AEtherC0r3

This comment has been minimized.

Copy link
Contributor Author

AEtherC0r3 commented Aug 5, 2017

Tracks#new
2017-08-06 00-44-55

Tracks#index
2017-08-06 00-45-26

Tracks#show
2017-08-06 00-46-04

admin/Tracks#index
2017-08-06 00-48-46

admin/Tracks#show for regular tracks
2017-08-06 01-03-17

admin/Tracks#show for self-organized tracks
2017-08-06 01-02-58

My Tracks in the user's menu
2017-08-06 01-04-25

Conference#show call for tracks in the splashpage
2017-08-06 00-57-55

@differentreality

This comment has been minimized.

Copy link
Contributor

differentreality commented Aug 6, 2017

👍 This looks good :D
I want to have another look before merging this, but in the meantime why don't you squash your commits?

@sayalilunkad what do you think? Anything to improve? Anything there might be missed?

@AEtherC0r3 AEtherC0r3 force-pushed the AEtherC0r3:track_request_acceptance_and_activation branch from 8695d66 to 6551294 Aug 7, 2017

@sayalilunkad
Copy link

sayalilunkad left a comment

Added a comment inline for one nit pick. Also once the admin decides what to do with the track when you try to accept it, it asks for the dates and venue. Shouldn't we add a sort of drop down for menu here? Also I am not able to put in the dates there. Can you attach a screenshot if it works for you?

If your track request is accepted, the conference organizers expect you to confirm that you will be able to hold it.
Then you will gain the Track organizer role.
%br
If your track request is rejected, you can either live with that or adapt it and resubmit it for review again.

This comment has been minimized.

Copy link
@sayalilunkad

sayalilunkad Aug 7, 2017

This doesn't sound very friendly. I would rephrase to If your track request is rejected, you have a chance to adapt it and resubmit it for review.

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Aug 8, 2017

Author Contributor

This is the same as in proposals. I thought of it as kinda playful, so I kept it like that. If you insist, I will change it

This comment has been minimized.

Copy link
@sayalilunkad

sayalilunkad Aug 8, 2017

Ah then let's keep it consistent!

@AEtherC0r3

This comment has been minimized.

Copy link
Contributor Author

AEtherC0r3 commented Aug 8, 2017

@sayalilunkad
We have a drop down to select the room
2017-08-08 09-53-09
If the conference doesn't have a venue or the venue doesn't have rooms then instead of the drop down it shows a message that informs the user to create a venue and rooms

Selecting a start date
2017-08-08 09-53-38

Selecting an end date
2017-08-08 09-54-11

@AEtherC0r3

This comment has been minimized.

Copy link
Contributor Author

AEtherC0r3 commented Aug 8, 2017

@sayalilunkad @differentreality Now, it is in bold and has links to admin_conference_venue and admin_conference_venue_rooms

@sayalilunkad

This comment has been minimized.

Copy link

sayalilunkad commented Aug 8, 2017

@AEtherC0r3 great thanks!

@AEtherC0r3

This comment has been minimized.

Copy link
Contributor Author

AEtherC0r3 commented Aug 8, 2017

2017-08-08 12-46-52

@AEtherC0r3

This comment has been minimized.

Copy link
Contributor Author

AEtherC0r3 commented Aug 8, 2017

@sayalilunkad You're welcome!

def track_selector_input(form)
if @program.tracks.any?
form.input :track_id, as: :select,
collection: @program.tracks.where(state: 'confirmed', cfp_active: true).pluck(:name, :id),

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

Why aren't you using the scopes here?

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Aug 9, 2017

Author Contributor

I didn't have the scopes when I initially wrote that code, and then I forgot about it

# Verify that the room is a room of the conference
def valid_room
if room && room.venue && room.venue.conference && program && program.conference && (program.conference != room.venue.conference)
errors.add(:room, "must be a room of #{program.conference.venue.name}")

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

Let's rewrite this:
return false unless room && room.venue etc
errors.add() if program.conference != room.venue.conference

transitions to: :new, from: [:rejected, :withdrawn, :canceled]
end
event :to_accept do
transitions to: :to_accept, from: [:new]

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

Can't we also change the state to to_accept, from to_reject?

@@ -66,4 +178,34 @@ def conference_id
def create_organizer_role
Role.where(name: 'track_organizer', resource: self).first_or_create(description: 'For the organizers of the Track')
end

def valid_dates

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

Let's make these validations a bit more readable.

= track.name
%td
= track.short_name
%td{style: "padding: 15px 0px 0px 10px;"}

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

Spaces within curly brackets

@@ -39,7 +39,7 @@
within('table#tracks') do
expect(page.has_content?(track.name)).to be false
expect(page.has_content?(track.description)).to be false
expect(page.assert_selector('tr', count: 1)).to be true
expect(page.has_content?('No data available in table')).to eq true

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

Also the feature test is very important to test if this is properly working for users that submit a request (non admin), but this is only testing it for organizers.

@@ -5,21 +5,35 @@
let!(:conference) { create(:conference, end_date: Date.today) }
let!(:cfp) { create(:cfp, start_date: Date.today - 2, end_date: Date.today - 1, program_id: conference.program.id) }

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

I would explicitly add the type here, and rename the variable accordingly.

end

it 'returns nil when the cfp for events doesn\'t exist' do
conference.program.cfp.destroy

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

I'd call the cfp for_events specifically here to avoid confusion.

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

I would also test in a different test that program.cfp returns the cfp for events.

This comment has been minimized.

Copy link
@AEtherC0r3

context 'excludes' do
%w[new to_accept accepted to_reject rejected canceled withdrawn].each do |state|
it "when track is #{state.humanize}" do

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

Perhaps we can rename the tests, so that it makes more sense to read it. Right now this will read:
Track scope #confirmed excludes when track is accepted

context 'returns true' do
context 'when self_organized? returns true' do
before :each do
allow(track).to receive(:self_organized?).and_return(true)

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

Why aren't you creating a self_organized track instead?

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Aug 15, 2017

Author Contributor

Because the method I'm testing here depends on the value of Track#self_organized? https://github.com/openSUSE/osem/blob/master/app/models/track.rb#L143. If I had created a self-organized track and, for example, something went wrong with self_organized? and it returned false, when it should have returned true, then this spec would fail. But, this spec isn't about self_organized? returning true for self-organized tracks and false for the regular ones, so, I'm forcing self_organized? to return a specific value in order to really test just Track#self_organized_and_accepted_or_confirmed? and nothing else. This way, when the test fails it most probably means that self_organized_and_accepted_or_confirmed? is behaving incorrectly

include RevisionCount

resourcify :roles, dependent: :delete_all

belongs_to :program
belongs_to :submitter, class_name: 'User'
belongs_to :room
has_many :events, dependent: :nullify

has_paper_trail only: [:name, :description, :color], meta: { conference_id: :conference_id }

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

Why only track changes for these attributes?

validates :start_date, presence: true, if: :self_organized_and_accepted_or_confirmed?
validates :end_date, presence: true, if: :self_organized_and_accepted_or_confirmed?
validates :room, presence: true, if: :self_organized_and_accepted_or_confirmed?
validates :relevance, presence: true, if: :self_organized?

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

What about the presence of description for self organized tracks?

@@ -6,59 +6,61 @@
Categorize events in your conference
.row
.col-md-12
%table.table.table-hover#tracks
%table.table.table-hover.table-striped.table-bordered.datatable#tracks
%thead
%th Name

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

We should add a field like ID, in order to be able to see which one is the oldest request, and which one is the newest.

%td
= link_to track.submitter.name, admin_user_path(track.submitter) if track.self_organized?
%td.text-center

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

This column is not sortable

%td.text-center
= check_box_tag "#{@conference.short_title}_#{track.short_name}", track.id, track.cfp_active,
class: 'switch-checkbox', method: :patch,
url: toggle_cfp_inclusion_admin_conference_program_track_path(@conference.short_title, id: track.short_name)+"?included=",

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 9, 2017

Contributor

When changing this, we have no indication about it, ie there is no flash message letting the users know if this happened, successfully or not.

@AEtherC0r3 AEtherC0r3 force-pushed the AEtherC0r3:track_request_acceptance_and_activation branch 3 times, most recently from e4c8ce4 to 49173cf Aug 9, 2017

@differentreality

This comment has been minimized.

Copy link
Contributor

differentreality commented Aug 10, 2017

👍 please squash the latest commits, so that we can merge it!

@AEtherC0r3 AEtherC0r3 force-pushed the AEtherC0r3:track_request_acceptance_and_activation branch from 49173cf to fbf8083 Aug 10, 2017

AEtherC0r3 added some commits Jul 12, 2017

Add room and dates to tracks
They are required only for accepted and confirmed self-organized tracks
Implement track request acceptance
Allow track submitter to request specific dates
Redirect to Tracks#edit if a track doesn't have a room or start/end date
before accepting it

Don't allow the submitter or the track organizers to edit the request
after it has been accepted or confirmed

Restrict track selection in proposals and move track selection from
Proposals form to events helper

Mark cfp_active of the tracks table as not null and fill in true if nil
Mark track state as not null and add default value
The regular tracks are marked as 'confirmed'
Change Cfp scopes to class methods
When no record matches the requested criteria a scope will return all
the records
In this case, if no record can be found we want the result to be nil

Also, make some readability fixes in specs
UI/UX changes related to tracks
Add confirmed and cfp_active scopes to Track
Render markdown in track's description
Make the views more similar to the proposal/events views
Change the sequence of columns in admin/Tracks#index
Remove the short_name column from the index views
Add button "My Tracks" in the user menu
Fix track count in proposals
Add details of track in admin/Tracks#show
Use tabs to show track details and events
Add relevance field for track requests
The requester can now provide more info about the track and himself
Track related fixes
Make the message in admin/Tracks form more visible by making it bold and
adding links to venue and rooms
Make papertrail track changes for all the track's attributes
Add validation to require presence of description for self-organized
tracks
Add ID column to admin/Tracks#index
Make the cfp inclusion column sortable
Show success/error flash messages after toggling cfp inclusion

@AEtherC0r3 AEtherC0r3 force-pushed the AEtherC0r3:track_request_acceptance_and_activation branch from fbf8083 to 4a29252 Aug 11, 2017

@differentreality
Copy link
Contributor

differentreality left a comment

Some comments for grooming and refactoring.

@@ -17,7 +17,7 @@ def set_timezone_for_this_request(&block)

def index
@events = @program.events
@tracks = @program.tracks
@tracks = @program.tracks.confirmed.cfp_active

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 11, 2017

Contributor

I am not sure we need the tracks to have cfp_active, so that we are able to move events from one track to another during evaluation period.
Let's look into that later on.

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Aug 14, 2017

Author Contributor

Fixed in #1631

- if can? :destroy, track
= link_to 'Delete', admin_conference_program_track_path(@conference.short_title, track),
method: :delete, class: 'btn btn-danger',
= link_to 'Delete', admin_conference_program_track_path(@conference.short_title, track), method: :delete, class: 'btn btn-danger',

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 11, 2017

Contributor

When is it possible to delete a track?
It could be when it is not self_organized, to keep things the same as before for conferences that don't have a cfp for tracks at all.

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Aug 12, 2017

Author Contributor

It's not possible to delete a track only when it is self-organized https://github.com/openSUSE/osem/blob/master/app/models/admin_ability.rb#L74-L76, hence the old behavior has been preserved

track_cfp_value = <%= @track.cfp_active %>;

track_cfp_td.attr('data-order', track_cfp_value);
$('#tracks').DataTable().cell(track_cfp_td).invalidate();

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 11, 2017

Contributor

I'd add a comment, as to what this line is doing, and why we need it.

@@ -10,13 +10,22 @@
- if @conference.splashpage and @conference.program.tracks.any? and @conference.splashpage.include_tracks
See rock-star speakers cover the topics of
- if @conference.splashpage and @conference.splashpage.include_tracks
- @conference.program.tracks.each_slice(3) do |slice|
- @conference.program.tracks.confirmed.cfp_active.each_slice(3) do |slice|

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 11, 2017

Contributor

What if the cfp_active attribute is false for a self_organized track, because its cfp ended sooner?

@@ -45,6 +45,10 @@
%section#program
= render 'schedule_splashpage'

- if @conference.program.cfps.for_tracks.try(:open?) && @conference.splashpage.include_cfp

This comment has been minimized.

Copy link
@differentreality

differentreality Aug 11, 2017

Contributor

Perhaps it makes sense to show this after cfp for events?

This comment has been minimized.

Copy link
@AEtherC0r3

AEtherC0r3 Aug 12, 2017

Author Contributor

Why? A conference is probably going to have the call for tracks before the call for papers, so that the tracks are available for proposal submissions

@differentreality

This comment has been minimized.

Copy link
Contributor

differentreality commented Aug 11, 2017

🎉 Yay we can get this in 🎉

@differentreality differentreality merged commit b717018 into openSUSE:master Aug 11, 2017

3 checks passed

Hakiri No security warnings were found.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.4%) to 84.422%
Details

@AEtherC0r3 AEtherC0r3 deleted the AEtherC0r3:track_request_acceptance_and_activation branch Aug 11, 2017

@AEtherC0r3

This comment has been minimized.

Copy link
Contributor Author

AEtherC0r3 commented Aug 17, 2017

The remaining comments have been addressed in #1639

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.