Closed volodymyr-chekyrta closed 3 months ago
Thanks for the pull request, @volodymyr-chekyrta! Please note that it may take us up to several weeks or months to complete a review and merge your PR.
Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.
Please let us know once your PR is ready for our review and all tests are green.
@saeedbashir, Kindly review the changes made in this pull request. Thank you 🙌
Please use the Firebase console for testing; here is a set of all the required fields for navigation:
Will review it tomorrow.
@volodymyr-chekyrta Do I need to test this PR against the sandbox because the API https://courses.edx.org/api/mobile/v4/notifications/create-token/ (404)
is returning 404
on the edX backend?
@volodymyr-chekyrta Do I need to test this PR against the sandbox because the API
https://courses.edx.org/api/mobile/v4/notifications/create-token/ (404)
is returning404
on the edX backend?
You cannot test this endpoint; it has not been merged yet. For now, please use the Firebase console.
Hi @saeedbashir, thank you for the prompt feedback! Feedback has been addressed, and PR is ready for the next pass 🙌
@saeedbashir, PR is ready for the next pass 🙌
@saeedbashir, PR is ready for the next pass 🙌
@volodymyr-chekyrta I've left few comments, please take a look. After that we are good to go with this PR.
@saeedbashir, PR is ready for the next pass 🙌
@volodymyr-chekyrta I've left few comments, please take a look. After that we are good to go with this PR.
@saeedbashir, thanks for the feedback; PR is ready for final pass 🚀
Remove the unused import
import Swinject
inFCMProvider.swift
and you are good to go 👍
Sure, done ✅
@rnr @saeedbashir approval was dismissed after the fix, could you please provide the approval and we'll merge it
@volodymyr-chekyrta 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.
This pull request introduces Firebase Cloud Messaging (FCM) to the project, enabling the functionality for receiving push notifications from Firebase.
Testing: • Verified push notifications are received when the app is in the foreground, background, and terminated states. • Confirmed the app correctly handles notification taps and displays relevant content. • Ensured token generation and handling work as expected, including handling token refreshes.
Please use the Firebase console for testing; here is a set of all the required fields for navigation:
Notifications:
enroll
course_id: Your course ID (example: course-v1:future+f1+2024)unenroll
add_beta_tester
course_id: Your course ID (example: course-v1:future+f1+2024)remove_beta_tester
course_id: Your course ID (example: course-v1:future+f1+2024)forum_response
topic_id: Your Topic ID (example: course) course_id: Your course ID (example: course-v1:future+f1+2024) thread_id: Your thread ID (example: 66584e96f2195c001ad219c7) comment_id: Your comment ID (example: 66585c9df2195c001ad219ce)forum_comment
topic_id: Your Topic ID (example: course) course_id: Your course ID (example: course-v1:future+f1+2024) thread_id: Your thread ID (example: 66583d13f2195c001a56a182) parent_id: Your response ID (example: 66586579f2195c001ad219d8) comment_id: Your comment ID (example: 6659ac92f2195c001bf37f77)