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
Image upload in creating issue #6962
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,4 +14,3 @@ def download | |
end | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,7 +60,6 @@ def show | |
|
||
def create | ||
@issue = Issues::CreateService.new(project, current_user, params[:issue]).execute | ||
|
||
respond_to do |format| | ||
format.html do | ||
if @issue.valid? | ||
|
@@ -69,15 +68,19 @@ def create | |
render :new | ||
end | ||
end | ||
format.js | ||
format.js do |format| | ||
# issue = Issues.create() | ||
|
||
@link = @issue.attachment.url.to_js | ||
end | ||
end | ||
end | ||
|
||
def update | ||
@issue = Issues::UpdateService.new(project, current_user, params[:issue]).execute(issue) | ||
|
||
respond_to do |format| | ||
format.js | ||
format.js | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace detected. |
||
format.html do | ||
if @issue.valid? | ||
redirect_to [@project, @issue] | ||
|
@@ -93,6 +96,21 @@ def bulk_update | |
redirect_to :back, notice: "#{result[:count]} issues updated" | ||
end | ||
|
||
def upload_image | ||
@upload_path = File.join(project.namespace.path, @project.path) | ||
@accepted_types = %w(png jpg jpeg gif) | ||
uploader = FileUploader.new('uploads/' + @upload_path, @accepted_types) | ||
links = [] | ||
params['issue-imgs'].each do |img| | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't it be simpler to limit the upload API to a single image (rather than several images at once), and let the javascript/browser send one request per image to upload? IMHO it would make the error handling easier on the browser side: each upload would receive its own success or error. If several files can be uploaded at once the error handling must deal with partial errors. |
||
alt = uploader.store!(img) | ||
links << {'alt' => File.basename(alt,'.*'), 'url' => File.join(root_url, uploader.url)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line is too long. [93/80] |
||
end | ||
|
||
respond_to do |format| | ||
format.json { render json: { links: links } } | ||
end | ||
end | ||
|
||
protected | ||
|
||
def issue | ||
|
@@ -128,7 +146,6 @@ def issues_filtered | |
# | ||
def redirect_old | ||
issue = @project.issues.find_by(id: params[:id]) | ||
|
||
if issue | ||
redirect_to project_issue_path(@project, issue) | ||
return | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,8 +15,12 @@ | |
# milestone_id :integer | ||
# state :string(255) | ||
# iid :integer | ||
# attachment :string(255) | ||
# | ||
|
||
require 'carrierwave/orm/activerecord' | ||
require 'file_size_validator' | ||
|
||
class Issue < ActiveRecord::Base | ||
include Issuable | ||
include InternalId | ||
|
@@ -30,7 +34,8 @@ class Issue < ActiveRecord::Base | |
scope :of_user_team, ->(team) { where(project_id: team.project_ids, assignee_id: team.member_ids) } | ||
|
||
attr_accessible :title, :assignee_id, :position, :description, | ||
:milestone_id, :label_list, :state_event | ||
:milestone_id, :label_list, :author_id_of_changes, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you add the |
||
:state_event | ||
|
||
acts_as_taggable_on :labels | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,14 +3,12 @@ class CreateService < Issues::BaseService | |
def execute | ||
issue = project.issues.new(params) | ||
issue.author = current_user | ||
|
||
if issue.save | ||
notification_service.new_issue(issue, current_user) | ||
event_service.open_issue(issue, current_user) | ||
issue.create_cross_references!(issue.project, current_user) | ||
execute_hooks(issue) | ||
end | ||
|
||
issue | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please avoid changes like this in files where you're not adding/changing code. |
||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,4 +33,4 @@ def file_storage? | |
def reset_events_cache(file) | ||
model.reset_events_cache if model.is_a?(User) | ||
end | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Final newline missing. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
# encoding: utf-8 | ||
class FileUploader < CarrierWave::Uploader::Base | ||
storage :file | ||
|
||
def initialize(path, allowed_extensions = nil) | ||
@path = path | ||
@allowed_extensions = allowed_extensions | ||
end | ||
|
||
def store_dir | ||
@path | ||
end | ||
|
||
def extension_white_list | ||
@allowed_extensions | ||
end | ||
|
||
def store!(file) | ||
original_filename = file.original_filename | ||
generate_filename(file) | ||
super | ||
original_filename | ||
end | ||
|
||
def generate_filename(file) | ||
new_filename = Digest::MD5.hexdigest(file.original_filename) | ||
file.original_filename = new_filename + File.extname(file.original_filename) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
= render "form" | ||
= render "form" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No newline? |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,6 +49,7 @@ | |
.wiki | ||
= preserve do | ||
= markdown @issue.description | ||
|
||
.context | ||
%cite.cgray | ||
= render partial: 'issue_context', locals: { issue: @issue } | ||
|
@@ -73,4 +74,4 @@ | |
= label.name | ||
| ||
|
||
.voting_notes#notes= render "projects/notes/notes_with_form" | ||
.voting_notes#notes= render "projects/notes/notes_with_form" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No newline? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this comment try to explain?