Skip to content

WhatsApp Dial Option #41

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

nathanielajayi7
Copy link

This PR gives users the option to add WhatsApp dial feature to the contact

Copy link
Owner

@fpalonso fpalonso left a comment

Choose a reason for hiding this comment

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

Hey @nathanielajayi7 , I sincerely appreciate your work here.
Unfortunately I do have some concerns about GDPR and other international laws when it comes to the way you're checking WhatsApp availability for a specific contact and I think we might need to explore other options. Therefore, while I thank you for your work, I'm not very confident about merging your PR at this moment.
I have also put a bunch of other comments that you might want to check.
Please let me know if you have any questions.
Thanks!

@@ -11,6 +11,6 @@
<entry name="!?*.kt" />
<entry name="!?*.clj" />
</wildcardResourcePatterns>
<bytecodeTargetLevel target="1.8" />
<bytecodeTargetLevel target="11" />
Copy link
Owner

Choose a reason for hiding this comment

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

Is this change actually needed for the feature?

@@ -45,7 +46,7 @@
</value>
</option>
</component>
<component name="ProjectRootManager" version="2" languageLevel="JDK_1_8" default="true" project-jdk-name="1.8" project-jdk-type="JavaSDK">
<component name="ProjectRootManager" version="2" languageLevel="JDK_11" project-jdk-name="1.8" project-jdk-type="JavaSDK">
Copy link
Owner

Choose a reason for hiding this comment

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

Do we really need to change this for this feature?

Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this is related to the feature.

Comment on lines +24 to +26
<package
android:name="com.whatsapp"
/>
Copy link
Owner

Choose a reason for hiding this comment

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

Could you please make this a single line?

<receiver android:name=".appwidget.DirectCallWidgetProvider">
<receiver
android:name=".appwidget.DirectCallWidgetProvider"
android:exported="true">
Copy link
Owner

Choose a reason for hiding this comment

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

Why do we need to export this receiver?



@SuppressLint("Range")
private fun hasWhatsapp(contactId:String, context: Context):String? {
Copy link
Owner

Choose a reason for hiding this comment

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

I have concerns about GPDR on this.

Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this one, the widget is already resizable.

Copy link
Owner

Choose a reason for hiding this comment

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

Do we need this one?

@@ -23,6 +23,7 @@ import android.net.Uri
data class Contact(
val displayName: String,
val photoUri: Uri?,
val contactId: String?,
Copy link
Owner

Choose a reason for hiding this comment

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

This one is WhatsApp-specific and shouldn't be part of this class.

Suggested change
val contactId: String?,

@@ -50,6 +52,7 @@ class ContactRepository(
return@withContext Contact(
getString(Contacts.DISPLAY_NAME) ?: "",
getUri(Contacts.PHOTO_URI),
getString(cursor.getColumnIndexOrThrow(ContactsContract.PhoneLookup._ID)),
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think we should risk getting an exception here.

@nathanielajayi7
Copy link
Author

Hey there, @fpalonso

Nice to see you're still active! I honestly didn't think you'd still see this PR.

Most of the latest fixes were last minute workarounds to get something done for my grandma (which is why I discovered your repo anyway lol)

I'll take every comment into consideration and work on them. If there are GPDR concerns about this feature, maybe it's also best it's forgone.

I'll do a little more digging and give you feedback

@fpalonso
Copy link
Owner

fpalonso commented Dec 14, 2024

Hey @nathanielajayi7 , sounds amazing, thanks for understanding.
I had indeed to pause my work here due to incompatibility with other projects, but I'm trying to get back at it.
I hope my feedback doesn't discourage you from keeping on collaborating on other issues of this project.
Also feel free to open any other issues that you think could help your grandma, that's the whole point of this app from the beginning :)
Thanks a lot!

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