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.

Sign up for GitHub

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

CV2-6041: Verify the relationship target before establishing a new one. #2201

Merged
merged 4 commits into from
Feb 5, 2025
Merged

CV2-6041: Verify the relationship target before establishing a new one. #2201

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a87a748
CV2-6041: check relationship target before creating new one
melsawy Feb 4, 2025
e4f6e57
CV2-6041: apply PR comments
melsawy Feb 4, 2025
573354f
CV2-6041: add more documentation
melsawy Feb 5, 2025
0f1a5a2
Merge branch 'develop' into CV2-6041-side-kiq-dead-jobs-active-record…
melsawy Feb 5, 2025
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
4 changes: 2 additions & 2 deletions app/models/concerns/smooch_messages.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,8 @@ def default_archived_flag
end

def save_message(message_json, app_id, author = nil, request_type = 'default_requests', associated_id = nil, associated_class = nil)
# associated: is the the "first media" and will be the source of the Relationship
# associated_obj: is used for TiplineRequest (smooch_resource_id field)
# associated: is the the "first media" and will be the source of the Relationship
# associated_obj: is used for TiplineRequest (smooch_resource_id field)
message = JSON.parse(message_json)
return if TiplineRequest.where(smooch_message_id: message['_id']).exists?
associated_obj = nil
Expand Down
1 change: 1 addition & 0 deletions app/models/relationship.rb
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

@melsawy maybe this is a dumb question: but why not have this logic after the if r.nil? in line 164? Why do we want to check for this in line 161 and then again in 164, but then do something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vasconsaurus the first check against source_id & target_id and also I need to check another case for target_id only to avoid DB validation (unique index for target_id)
I did this before L164 as I need to skip any operation in case there is a relation with same target_id

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the explanation @melsawy!

Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ def create_or_update_parent_id

def self.create_unless_exists(source_id, target_id, relationship_type, options = {})
r = Relationship.where(source_id: source_id, target_id: target_id).where('relationship_type = ?', relationship_type.to_yaml).last
r = Relationship.where(target_id: target_id).last if r.nil?
exception_message = nil
exception_class = nil
if r.nil?
Expand Down
Loading