Skip to content

FEATURE: Can edit category/host relationships for embedding#10

Open
ShashankFC wants to merge 1 commit into
rest-serializer-enhancement-prefrom
rest-serializer-enhancement-post
Open

FEATURE: Can edit category/host relationships for embedding#10
ShashankFC wants to merge 1 commit into
rest-serializer-enhancement-prefrom
rest-serializer-enhancement-post

Conversation

@ShashankFC

Copy link
Copy Markdown
Collaborator

Comment on lines +9 to +11
category_id = execute("SELECT c.id FROM categories AS c
INNER JOIN site_settings AS s ON s.value = c.name
WHERE s.name = 'embed_category'")[0]['id'].to_i

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This query will crash if no matching category is found, as it tries to access [0]['id'] on a potentially empty result set. Needs nil checking.

result = execute("SELECT c.id FROM categories AS c...")
category_id = result.first ? result.first['id'].to_i : 0
Suggested change
category_id = execute("SELECT c.id FROM categories AS c
INNER JOIN site_settings AS s ON s.value = c.name
WHERE s.name = 'embed_category'")[0]['id'].to_i
result = execute("SELECT c.id FROM categories AS c
INNER JOIN site_settings AS s ON s.value = c.name
WHERE s.name = 'embed_category'")
category_id = result.first ? result.first['id'].to_i : 0

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Comment on lines +5 to +8
before_validation do
self.host.sub!(/^https?:\/\//, '')
self.host.sub!(/\/.*$/, '')
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Calling sub! on self.host will raise NoMethodError if host is nil. Needs nil check before mutation.

before_validation do
  if self.host.present?
    self.host.sub!(/^https?:\/\//, '')
    self.host.sub!(/\/.*$/, '')
  end
end
Suggested change
before_validation do
self.host.sub!(/^https?:\/\//, '')
self.host.sub!(/\/.*$/, '')
end
before_validation do
if self.host.present?
self.host.sub!(/^https?:\/\//, '')
self.host.sub!(/\/.*$/, '')
end
end

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Comment on lines +10 to +11
host = EmbeddableHost.where(id: params[:id]).first
save_host(host)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If no host is found with the given ID, host will be nil and calling save_host(nil) will cause NoMethodError when accessing nil properties.

def update
  host = EmbeddableHost.where(id: params[:id]).first
  return render_json_error('Host not found', status: 404) unless host
  save_host(host)
end
Suggested change
host = EmbeddableHost.where(id: params[:id]).first
save_host(host)
host = EmbeddableHost.where(id: params[:id]).first
return render_json_error('Host not found', status: 404) unless host
save_host(host)

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

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.

2 participants