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

[REFACTOR]: [Tsys] Remove Default Case Handling #2288

Closed
2 tasks done
Sakilmostak opened this issue Sep 22, 2023 · 11 comments · Fixed by #2672
Closed
2 tasks done

[REFACTOR]: [Tsys] Remove Default Case Handling #2288

Sakilmostak opened this issue Sep 22, 2023 · 11 comments · Fixed by #2672
Assignees
Labels
A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement good first issue Good for newcomers hacktoberfest Issues that are up for grabs for Hacktoberfest participants

Comments

@Sakilmostak
Copy link
Contributor

📝 Feature Description

  • We utilize match statements to make pivotal decisions, such as generating requests based on the payment method type and managing responses received from the connector.
  • These conditions generally go hand in hand with enum variants.
  • Default case is used because a match statement needs to be exhaustive i.e. every variant needs to be covered.
  • So, if all the explicit cases are handled then default is used to handle the rest.
  • Each connector have these match statements but many of them don’t provide reference to each variant in their default case, rather a _ is put to handle all the other cases.
  • This approach carries a risk because developers may inadvertently overlook the need for explicit handling of the new cases.

🔨 Possible Implementation

🔖 Note: All the changes needed should be contained within hyperswitch/crates/router/src/connector/

📦 Have you spent some time checking if this feature request has been raised before?

  • I checked and didn't find a similar issue

📦 Have you read the Contributing Guidelines?

✨ Are you willing to submit a PR?

@Sakilmostak Sakilmostak added A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement S-awaiting-triage Status: New issues that have not been assessed yet hacktoberfest Issues that are up for grabs for Hacktoberfest participants labels Sep 22, 2023
@VedantKhairnar VedantKhairnar added the good first issue Good for newcomers label Sep 28, 2023
@Harshpal007
Copy link

Harshpal007 commented Oct 4, 2023

I would like to contribute on this request.

@AkshayaFoiger AkshayaFoiger removed the S-awaiting-triage Status: New issues that have not been assessed yet label Oct 4, 2023
@AkshayaFoiger
Copy link
Contributor

Hey @Harshpal007, sure! I've assigned this to you.

In case you have any queries, you can ask them on this issue thread, or on our discord server, or on slack whichever you are comfortable with

@VedantKhairnar
Copy link
Collaborator

Hey @Harshpal007 ,
Let us know if you face any issues. Happy to help! : )

@Harshpal007
Copy link

Hey @Harshpal007 ,
Let us know if you face any issues. Happy to help! : )

Hey thanks for reaching out,
I have understood the problem and know what to do,but facing an issue with where I should implement the solution as there are multiple paying methods.Should I implement the changes individually in the methods or should I try finding a unified solution.

@AkshayaFoiger
Copy link
Contributor

Hi @Harshpal007, you have to implement the necessary modifications wherever the match arm contains a "_" value in the following file hyperswitch_oss/crates/router/src/connector/tsys/transformers.rs
Thankyou.

@VedantKhairnar
Copy link
Collaborator

Hey @Harshpal007 ,
Just checking in - are you working on it?
Let us know if you need any help.

@Rutam21
Copy link
Contributor

Rutam21 commented Oct 18, 2023

@VedantKhairnar I would like to take this issue as there's been no activity on this for a while now. Also, please point out where I can refer to find the list of all the cases to be handled. Thanks.

@VedantKhairnar
Copy link
Collaborator

@Rutam21 At this moment, 2 issues are assigned to you. Once either of them is merged, will assign this one to you. Works?

@Rutam21
Copy link
Contributor

Rutam21 commented Oct 19, 2023

Yeah, that works. One of the PRs is already raised and approved. Waiting for another approval before merge.

@Harshwardhan9431
Copy link
Contributor

@VedantKhairnar Can you assign this issue to me.

@SanchithHegde
Copy link
Member

SanchithHegde commented Oct 22, 2023

Sure @Harshwardhan9431!

Edit: Un-assigning @Harshpal007 due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-connector-integration Area: Connector integration C-feature Category: Feature request or enhancement good first issue Good for newcomers hacktoberfest Issues that are up for grabs for Hacktoberfest participants
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants