add platform_medium field to Conversation model #707
Conversation
WalkthroughA new field named 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🪛 Pylint (3.3.7)bots/migrations/0104_conversation_platform_medium.py[refactor] 6-6: Too few public methods (0/2) (R0903) bots/models/convo_msg.py[refactor] 35-35: Too few public methods (0/2) (R0903) ⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Review Summary🏷️ Draft Comments (2)
|
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
daras_ai_v2/twilio_bot.py (1)
28-33:platform_mediumin lookup will duplicate conversationsPassing
platform_medium="SMS"directly toget_or_createmakes it part of the lookup
criteria. Existing SMS conversations created before this change (with an empty value)
will no longer match – a new row is created every message.- self.convo = Conversation.objects.get_or_create( - bot_integration=bi, - twilio_phone_number=data["From"][0], - twilio_call_sid="", - platform_medium="SMS", - )[0] + self.convo, created = Conversation.objects.get_or_create( + bot_integration=bi, + twilio_phone_number=data["From"][0], + twilio_call_sid="", + defaults={"platform_medium": "SMS"}, + ) + if not created and self.convo.platform_medium != "SMS": + self.convo.platform_medium = "SMS" + self.convo.save(update_fields=["platform_medium"])Prevents duplicate rows and retro-fills the new field for legacy conversations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
bots/migrations/0104_conversation_platform_medium.py(1 hunks)bots/models/convo_msg.py(1 hunks)daras_ai_v2/bots.py(1 hunks)daras_ai_v2/twilio_bot.py(2 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
bots/migrations/0104_conversation_platform_medium.py
[refactor] 6-6: Too few public methods (0/2)
(R0903)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (python)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (3)
bots/models/convo_msg.py (1)
241-245: PreferCharField+ choices over unrestrictedTextFieldfor a finite set
platform_mediumappears to take on only a handful of values ("SMS","Voice", maybe"MMS"in future).
UsingTextField(unbounded, no validation) risks accidental typos and wastes space.-platform_medium = models.TextField( - blank=True, - default="", - help_text="Platform medium Voice / SMS ", -) +class PlatformMedium(models.TextChoices): + SMS = "SMS", "SMS" + VOICE = "Voice", "Voice" + +platform_medium = models.CharField( + max_length=10, + choices=PlatformMedium.choices, + blank=True, + default="", + help_text="Platform medium (Voice / SMS)", +)Keeps storage small, enforces valid values and improves admin UX.
[ suggest_optional_refactor ]daras_ai_v2/bots.py (1)
592-593: LGTM – new variable correctly surfaced to recipesThe Twilio-only branch now exposes
platform_medium; this slots neatly into the existing schema generation logic.
No further action required.bots/migrations/0104_conversation_platform_medium.py (1)
1-20: Migration looks correct but remember to back-fill existing rowsThe new column defaults to
'', so historical Twilio conversations will keep an empty value.
If analytics or recipes rely on a non-emptyplatform_medium, consider a data-migration to set:UPDATE bots_conversation SET platform_medium = 'SMS' WHERE platform = 'TWILIO' AND twilio_call_sid = ''; UPDATE bots_conversation SET platform_medium = 'Voice' WHERE platform = 'TWILIO' AND twilio_call_sid <> '';(Not needed if downstream code gracefully handles empty strings.)
milovate
force-pushed
the
convo-platform-medium
branch
from
294744f to
452471a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bots/migrations/0104_conversation_platform_medium.py (1)
16-18: Nit: polish help textAdd parentheses or a verb for clarity.
-help_text="Platform medium Voice / SMS" +help_text="Platform medium (Voice / SMS)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
bots/migrations/0104_conversation_platform_medium.py(1 hunks)bots/models/convo_msg.py(1 hunks)daras_ai_v2/bots.py(1 hunks)daras_ai_v2/twilio_bot.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- daras_ai_v2/bots.py
- daras_ai_v2/twilio_bot.py
- bots/models/convo_msg.py
🧰 Additional context used
🪛 Pylint (3.3.7)
bots/migrations/0104_conversation_platform_medium.py
[refactor] 6-6: Too few public methods (0/2)
(R0903)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (3.10.12, 1.8.3)
bots/models/convo_msg.py
Outdated
| @@ -238,6 +238,12 @@ class Conversation(models.Model): | |||
| help_text="Twilio call sid (only used if each call is a new conversation)", | |||
| ) | |||
|
|
|||
| platform_medium = models.TextField( | |||
There was a problem hiding this comment.
It is best to use an integer enum (or at least a CharField with choices) for platform_medium instead of a free-form TextField.
Why:
- Data integrity: An int enum restricts values to a known set, preventing typos and inconsistent data (e.g., 'voice', 'Voice', 'VOICE', 'sms', etc.).
- Performance: Integer fields are more efficient for storage and queries than text fields, especially as the table grows.
- Validation: Enums make it easy to validate and enforce allowed values at both the database and application level.
- Admin UX: Django admin and forms will automatically provide dropdowns for enums, making it clear what values are valid.
- Future-proofing: If you ever need to add more mediums, you can do so in a controlled way without risking legacy data issues.
A TextField allows any string, which can lead to data quality problems and makes analytics or filtering harder. Using an enum ensures consistency and reliability.
milovate
force-pushed
the
convo-platform-medium
branch
from
452471a to
7cf9c2a
Compare
Q/A checklist
You can visualize this using tuna:
To measure import time for a specific library:
To reduce import times, import libraries that take a long time inside the functions that use them instead of at the top of the file:
Legal Boilerplate
Look, I get it. The entity doing business as “Gooey.AI” and/or “Dara.network” was incorporated in the State of Delaware in 2020 as Dara Network Inc. and is gonna need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Dara Network Inc can use, modify, copy, and redistribute my contributions, under its choice of terms.