Skip to content

Commit b22c893

Browse files
committed
Clean up note reference when deleting a reply
Deleting a reply only removed its own triples, leaving the parent note still pointing at the now-deleted reply and orphaning any child replies. DELETE /replies/:id now detaches the reply from every note that references it, recursively deletes child replies, and returns 404 instead of crashing when the reply does not exist. Add tests for the note-reference cleanup and the 404 case.
1 parent cedb67e commit b22c893

2 files changed

Lines changed: 75 additions & 1 deletion

File tree

controllers/replies_controller.rb

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,35 @@ class RepliesController < ApplicationController
7878
# Delete a reply
7979
delete '/:replyid' do
8080
_reply = LinkedData::Models::Notes::Reply.find(params["replyid"]).first
81-
_reply.delete
81+
error 404, "Reply #{params["replyid"]} not found" if _reply.nil?
82+
remove_reply(_reply)
8283
halt 204
8384
end
85+
86+
# Detach the reply from any note that references it and recursively delete
87+
# its child replies before deleting the reply itself, so that no dangling
88+
# reference to the deleted reply is left behind.
89+
def remove_reply(reply)
90+
notes_referencing(reply).each do |note|
91+
note.reply = note.reply.reject { |r| r.id == reply.id }
92+
note.save
93+
end
94+
95+
reply.bring(:children) unless reply.loaded_attributes.include?(:children)
96+
reply.children.each { |child| remove_reply(child) }
97+
98+
reply.delete
99+
end
100+
101+
def notes_referencing(reply)
102+
reply_predicate = LinkedData::Models::Note.attribute_uri(:reply)
103+
Goo.sparql_query_client
104+
.select(:id).distinct
105+
.from(LinkedData::Models::Note.uri_type)
106+
.where([:id, reply_predicate, reply.id])
107+
.each_solution
108+
.map { |sol| LinkedData::Models::Note.find(sol[:id]).include(LinkedData::Models::Note.attributes).first }
109+
.compact
110+
end
84111
end
85112
end

test/controllers/test_replies_controller.rb

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,51 @@ def test_reply_lifecycle
7979
assert last_response.status == 204
8080
end
8181

82+
def test_delete_reply_removes_note_reference
83+
note = LinkedData::Models::Note.new({
84+
creator: @@user,
85+
subject: "Note for delete reference test",
86+
body: "Body for delete reference test",
87+
relatedOntology: [@@ontology]
88+
})
89+
note.save
90+
91+
reply = LinkedData::Models::Notes::Reply.new({
92+
creator: @@user,
93+
body: "Reply to be deleted"
94+
})
95+
reply.save
96+
97+
child = LinkedData::Models::Notes::Reply.new({
98+
creator: @@user,
99+
body: "Child of the deleted reply",
100+
parent: reply
101+
})
102+
child.save
103+
104+
note.reply = (note.reply || []).dup.push(reply)
105+
note.save
106+
107+
reply_id = reply.id
108+
child_id = child.id
109+
110+
delete reply_id.to_s
111+
assert_equal 204, last_response.status
112+
113+
# The reply and its child are gone
114+
assert_nil LinkedData::Models::Notes::Reply.find(reply_id).first
115+
assert_nil LinkedData::Models::Notes::Reply.find(child_id).first
116+
117+
# The parent note no longer references the deleted reply
118+
note_after = LinkedData::Models::Note.find(note.id).include(:reply).first
119+
refute_includes (note_after.reply || []).map { |r| r.id }, reply_id
120+
121+
note_after.delete
122+
end
123+
124+
def test_delete_missing_reply_returns_404
125+
delete "/replies/does-not-exist"
126+
assert_equal 404, last_response.status
127+
end
128+
82129
end

0 commit comments

Comments
 (0)