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

Add note type for activities #1103

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions app/controllers/admin/activities_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ def show
end

def create
@activity = Activity.new(activity_post_params.except(:_destroy))
@activity = Activity.new(activity_post_params.except(:_destroy, :notes_options))

if @activity.notes_input_type != 'text' && params[:activity][:notes_options].present?
@activity.notes = params[:activity][:notes_options].compact_blank.join("\n")
end
if @activity.save
# manual call to impressionist, because otherwise the activity doesn't have an id yet
impressionist(@activity)
Expand Down Expand Up @@ -63,13 +66,16 @@ def update
@activity = Activity.find(params[:id])
params = activity_post_params

if params[:notes_input_type] != 'text' && params[:notes_options].present?
params[:notes] = params[:notes_options].compact_blank.join("\n")
end
# removing the images from disk
if params[:_destroy] == 'true'
logger.debug('remove poster from activity')
@activity.poster.purge
end

if @activity.update(params.except(:_destroy))
if @activity.update(params.except(:_destroy, :notes_options))
redirect_to(@activity)
else
@recipients = @activity.payment_mail_recipients
Expand Down Expand Up @@ -119,6 +125,8 @@ def activity_post_params
:is_seniors,
:participant_limit,
:show_participants,
:_destroy)
:notes_input_type,
:_destroy,
notes_options: [])
end
end
3 changes: 2 additions & 1 deletion app/controllers/members/participants_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ def create
end

@member = Member.find(current_user.credentials_id)
@notes = params[:par_notes]
@notes = params[:par_notes]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did the linter add a double space? Linters 🤷


reservist = false

# Deny if already enrolled
Expand Down
107 changes: 107 additions & 0 deletions app/javascript/src/admin/activities.js
Original file line number Diff line number Diff line change
Expand Up @@ -540,3 +540,110 @@ function posterHandlers() {
$(this).find('button[type="submit"].wait').addClass("disabled");
});
}

$(document).on("click", "#add-notes-option", function () {
var optionIndex = $("#notes-options .col-md-6").length;
var optionHtml =
'<div class="col-md-6">' +
'<div class="input-group mb-3">' +
'<input class="form-control" type="text" name="activity[notes_options][]">' +
'<div class="input-group-append">' +
'<button class="btn btn-sm btn-danger delete-option" type="button">' +
'<i class="fa fa-fw fa-minus"></i>' +
"</button>" +
"</div>" +
"</div>" +
"</div>";
$("#notes-options").append(optionHtml);
});

$(document).on("click", ".delete-option", function () {
$(this).closest(".col-md-6").remove();
});

$(document).on("change", "#activity_notes_input_type", function () {
var notesInputType = $(this).val();
var notesContainer = $("#notes-container");

if (notesInputType === "text") {
var notesValue = $("#notes-options input")
.map(function () {
return $(this).val();
})
.get()
.join("\n");
notesContainer.html(
'<textarea class="form-control" id="activity_notes" name="activity[notes]">' +
notesValue +
"</textarea>",
);
} else {
var optionsHtml = "";
var notesValue = $("#activity_notes").val();
var optionsArray = [];

if (notesValue && notesValue.trim() !== "") {
optionsArray = notesValue.split("\n");
}

if (optionsArray.length > 0) {
var titleValue = optionsArray[0];
optionsHtml +=
'<div class="col-md-12">' +
'<div class="input-group mb-3">' +
'<span class="input-group-text">Title</span>' +
'<input class="form-control" type="text" name="activity[notes_options][]" value="' +
titleValue +
'">' +
"</div>" +
"</div>";

optionsArray.slice(1).forEach(function (optionValue) {
optionsHtml +=
'<div class="col-md-6">' +
'<div class="input-group mb-3">' +
'<input class="form-control" type="text" name="activity[notes_options][]" value="' +
optionValue +
'">' +
'<div class="input-group-append">' +
'<button class="btn btn-sm btn-danger delete-option" type="button">' +
'<i class="fa fa-fw fa-minus"></i>' +
"</button>" +
"</div>" +
"</div>" +
"</div>";
});
} else {
optionsHtml +=
'<div class="col-md-12">' +
'<div class="input-group mb-3">' +
'<span class="input-group-text">Title</span>' +
'<input class="form-control" type="text" name="activity[notes_options][]">' +
"</div>" +
"</div>" +
'<div class="col-md-6">' +
'<div class="input-group mb-3">' +
'<input class="form-control" type="text" name="activity[notes_options][]">' +
'<div class="input-group-append">' +
'<button class="btn btn-sm btn-danger delete-option" type="button">' +
'<i class="fa fa-fw fa-minus"></i>' +
"</button>" +
"</div>" +
"</div>" +
"</div>";
Comment on lines +618 to +633
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps add some indentation here?

Copy link
Contributor Author

@Siem2l Siem2l Jun 5, 2024

Choose a reason for hiding this comment

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

Might be preferable, however, an even better approach would be to eliminate the need for generating HTML within the JavaScript code altogether.

Make a HAML partial file as a template for the option menu.
In the _edit haml file render the template but keep it hidden with css attributes
Update the code that appends the options HTML to use the cloned template.
Modify the code that sets the input values to target the cloned elements and make it visible
Add an event listener for the "Add Option" button to clone and append the template.
Add an event listener for the "Delete Option" button to remove the corresponding option.

this is not required, but would prevent coding twice when changing the UI

}

notesContainer.html(
'<div class="row" id="notes-options">' +
optionsHtml +
"</div>" +
'<div class="row">' +
'<div class="col-md-12">' +
'<button class="btn btn-primary" id="add-notes-option" type="button">' +
'<i class="fa fa-fw fa-plus"></i> Add Option' +
"</button>" +
"</div>" +
"</div>",
);
}
});
34 changes: 32 additions & 2 deletions app/javascript/src/members/activities/activity.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,19 @@ export class Activity {
has_notes() {
return this.notes.length !== 0;
}

has_notes_checkboxes() {
return find_in_object(this.notes, function (note) {
return note.type === "checkbox";
});
}

has_notes_radio_buttons() {
return find_in_object(this.notes, function (note) {
return note.type === "radio";
});
}

are_notes_filled() {
return $.trim(this.notes.val()).length > 0;
}
Expand Down Expand Up @@ -184,12 +197,29 @@ Object.defineProperties(Activity.prototype, {
*/
value: function (method) {
var activity = this;
var par_notes;
var notes = [];
console.log(activity.has_notes_checkboxes);
Copy link
Member

@SilasPeters SilasPeters Jun 4, 2024

Choose a reason for hiding this comment

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

Shouldn't the console log be removed?

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

if (activity.has_notes_checkboxes()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I also know little JS, however, does it not have switch cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any way to define the enum in 1 place like in activity.rb and reference it from the jabbascript?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think so, atleast not trivally

console.log(this.notes.find(":checked"));
Copy link
Contributor

Choose a reason for hiding this comment

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

remove log or cache this.notes call

this.notes.each(function () {
if (this.checked) {
notes.push(this.value);
}
});
par_notes = notes.join(", ");
} else if (activity.has_notes_radio_buttons()) {
par_notes = this.notes.find(":checked").val();
} else {
par_notes = this.notes.val();
}

var request = $.ajax({
url: "/activities/" + activity.id + "/participants",
type: method,
data: {
authenticity_token: this.token,
par_notes: this.notes.val(),
par_notes: par_notes,
},
})
.done(function (response) {
Expand Down Expand Up @@ -374,7 +404,7 @@ Object.defineProperties(
},

notes: function () {
return this.panel.find(".notes");
return this.panel.find(".notes"); // .find function returns a jQuery object with the found elements
},

notes_mandatory: function () {
Expand Down
2 changes: 1 addition & 1 deletion app/models/activity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ def content_type
end
end

enum notes_input_type: { text: 0, checkboxes: 1, radio_buttons: 2 }
validates :notes, presence: true, if: proc { |a| a.notes_public? || a.notes_mandatory? }

is_impressionable

before_validation :validate_enrollable
Expand Down
48 changes: 44 additions & 4 deletions app/views/admin/activities/partials/_edit.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,56 @@
= label(:activity, :organized_by)
.ui-select
= f.select :organized_by, options_for_select(Group.has_members.order(:category, :name).map{ |group| [group.name, group.id] }, @activity.organized_by), :include_blank => '-'

.form-group
.row
.col-md-12
= f.label :notes

.row
.col-md-12#notes-container
- if @activity.notes_input_type == 'text'
= f.text_area :notes, value: @activity.notes, class: 'form-control', id: 'activity-notes'
- elsif @activity.notes_input_type == 'checkboxes' || @activity.notes_input_type == 'radio_buttons'
.row#notes-options
- if @activity.notes.present?
- notes_lines = @activity.notes.split("\n")
- if notes_lines.length > 1
- title = notes_lines.first
.col-md-12
.input-group.mb-3
%span.input-group-text Title
= text_field_tag "activity[notes_options][]", title, class: 'form-control'
- notes_lines.drop(1).each do |option|
.col-md-6
.input-group.mb-3
= text_field_tag "activity[notes_options][]", option, class: 'form-control'
.input-group-append
%button.btn.btn-sm.btn-danger.delete-option{ type: 'button' }
%i.fa.fa-fw.fa-minus
- else
.col-md-12
.input-group.mb-3
%span.input-group-text Title
= text_field_tag "activity[notes_options][]", @activity.notes, class: 'form-control'
- else
.col-md-12
.input-group.mb-3
%span.input-group-text Title
= text_field_tag "activity[notes_options][]", '', class: 'form-control'
.col-md-6
.input-group.mb-3
= text_field_tag "activity[notes_options][]", '', class: 'form-control'
.input-group-append
%button.btn.btn-sm.btn-danger.delete-option{ type: 'button' }
%i.fa.fa-fw.fa-minus
.row
.col-md-12
%button.btn.btn-primary#add-notes-option{ type: 'button' }
%i.fa.fa-fw.fa-plus
Add Option
.row
.col-md-12
= f.text_field :notes, :value => @activity.notes, :class => 'form-control', id: "activity-notes"

= f.label :notes_input_type
= f.select :notes_input_type, Activity.notes_input_types.keys.map { |type| [type.humanize, type] }, {}, class: 'form-control'
.row
.col-sm-6
= f.check_box :notes_public, checked: @activity.notes_public
Expand Down
Loading
Loading