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.

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

Draft: Add Vodacom Mobile Money integration package #520

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

beesaferoot
Copy link
Contributor

Brief summary of the change made

Are there any other side effects of this change that we should be aware of?

Describe how you tested your changes?

Pull Request checklist

Please confirm you have completed any of the necessary steps below.

  • Meaningful Pull Request title and description
  • Changes tested as described above
  • Added appropriate documentation for the change.
  • Created GitHub issues for any relevant followup/future enhancements if appropriate.

@beesaferoot beesaferoot changed the title Add Vodacom Mobile Money integration package Draft: Add Vodacom Mobile Money integration package Feb 27, 2025
@beesaferoot
Copy link
Contributor Author

This is a Draft PR to setup a plugin. Your reviews are welcome

@dmohns @93Kamuran

@dmohns
Copy link
Member

dmohns commented Feb 27, 2025

Nice @beesaferoot I like it a lot!!

@93Kamuran could you please review the Plugin Development guide? And let us know that this is the process you currently follow when developing a plugin for MPM?

@dmohns
Copy link
Member

dmohns commented Feb 27, 2025

@beesaferoot

The goal this is to move the Vodacom dummy endpoints into a plugin, right? I would have expected this PR to then remove the code that we added previously, no?

@beesaferoot
Copy link
Contributor Author

beesaferoot commented Feb 27, 2025

@beesaferoot

The goal this is to move the Vodacom dummy endpoints into a plugin, right? I would have expected this PR to then remove the code that we added previously, no?

Yeah, I will be doing so with this. I wanted to get feedback first before going too deep.

Copy link
Contributor

@93Kamuran 93Kamuran left a comment

Choose a reason for hiding this comment

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

Looks good for a draft. Let's wait until our next meeting with Vodacom IT team then fix/update required areas.

@@ -2,9 +2,7 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

We will remove this controller. In #526

@@ -0,0 +1,19 @@
<?php
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we're gonna resolve companyId from JTW token in request instead from url segment. However, it's okay for draft.

@@ -0,0 +1,24 @@
<?php
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a status column here, that could be an enum as ['pending','succeeded','failed'] or [0,1,-1]

@@ -332,9 +332,3 @@ static function () {
Route::group(['prefix' => 'usage-types'], static function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

these routes will have been removed in #526

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.

3 participants