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

[BUG] Exporter enum types incorrect #494

Closed
tomdebuyser opened this issue Jun 15, 2020 · 14 comments
Closed

[BUG] Exporter enum types incorrect #494

tomdebuyser opened this issue Jun 15, 2020 · 14 comments
Assignees
Labels
bug Something isn't working Feature New feature or request wontfix This will not be worked on

Comments

@tomdebuyser
Copy link

tomdebuyser commented Jun 15, 2020

Describe the bug
The enum types that are exported in notificationRequestItem.d.ts (and probably other files that I do not use yet) are not correct. They only provide a enum name without value. As you can see in the TypeScript enum docs (https://www.typescriptlang.org/docs/handbook/enums.html#numeric-enums), when declaring an enum without values, it will result in a number, while the Adyen API is returning strings.

To Reproduce
Install @adyen/api-library and open notificationRequestItem.d.ts.

Expected behavior
Those enums have a string value, representing the value that is sent from the Adyen API. I created some typings in my own project to get around this bug:

enum NotificationEventCode {
    AUTHORISATION = 'AUTHORISATION',
    CANCELLATION = 'CANCELLATION',
    CANCEL_OR_REFUND = 'CANCELORREFUND',
    REFUND = 'REFUND',
    REFUND_FAILED = 'REFUNDFAILED',
}

enum NotificationSuccess {
    True = 'true',
    False = 'false',
}

Screenshots

export declare namespace NotificationRequestItem {
    enum EventCodeEnum {
        AUTHORISATION,
        AUTHORISATIONADJUSTMENT,
        CANCELLATION,
        CANCELORREFUND,
        CAPTURE,
        CAPTUREFAILED,
        HANDLEDEXTERNALLY,
        ORDEROPENED,
        ORDERCLOSED,
        PENDING,
        PROCESSRETRY,
        REFUND,
        REFUNDFAILED,
        REFUNDEDREVERSED,
        REFUNDWITHDATA,
        REPORTAVAILABLE,
        VOIDPENDINGREFUND,
        CHARGEBACK,
        CHARGEBACKREVERSED,
        NOTIFICATIONOFCHARGEBACK,
        NOTIFICATIONOFFRAUD,
        PREARBITRATIONLOST,
        PREARBITRATIONWON,
        REQUESTFORINFORMATION,
        SECONDCHARGEBACK,
        PAYOUTEXPIRE,
        PAYOUTDECLINE,
        PAYOUTTHIRDPARTY,
        PAIDOUTREVERSED,
        AUTORESCUE,
        CANCELAUTORESCUE,
        RECURRINGCONTRACT,
        POSTPONEDREFUND,
        OFFERCLOSED,
        MANUALREVIEWACCEPT,
        MANUALREVIEWREJECT
    }
    enum OperationsEnum {
        CANCEL,
        CAPTURE,
        REFUND
    }
    enum SuccessEnum {
        True,
        False
    }
}

Desktop (please complete the following information):

  • OS: Mac
  • Node Version: 12.16.3
  • NPM Version: 6.14.4
@tomdebuyser tomdebuyser added the bug Something isn't working label Jun 15, 2020
@tomdebuyser
Copy link
Author

tomdebuyser commented Jun 15, 2020

At the moment I am using those typings in my project to handle webhook notifications. To be able to use them, I have to import/require them from the dist folder, because the typings are not exported from the index file.

Is there a reason why they are not exported?

Current import:

import {
    Notification,
    NotificationRequestItem,
} from '@adyen/api-library/dist/lib/src/typings/notification/models'

Expected import would be inside these imports:

import {
    Client,
    Config,
    CheckoutAPI,
    Modification,
    hmacValidator,
} from '@adyen/api-library';

@KadoBOT
Copy link
Contributor

KadoBOT commented Jun 15, 2020

Hi @tomdebuyser

We have plans to move our types definitions to @types so they can be used on other typescript projects. Currently, the types are inherited when you use any of the public API's methods and not meant to be used alone.

But I think it makes sense to expose the NotificationRequest.

Can you check if passing Notification to the NotificationRequest Object solves your issue?

@tomdebuyser
Copy link
Author

Thanks for your answer @KadoBOT

I don't really see how this will solve the issue, the enum values will still be invalid? Also types like NotificationItem containing 'notificationRequestItem': NotificationRequestItem is leading to bugs, because in reality the notificationRequestItem is starting with a capital N when receiving the notification as a json.

Creating an @types library with correct typings would definitely help. I know importing from the dist folder is not a good practice, but I explicitly have to type things any to make my compiler work, because it expects certain types that are defined in the dist folder.

@KadoBOT
Copy link
Contributor

KadoBOT commented Jun 15, 2020

@tomdebuyser yes, it is expected that NotificationRequestItem has capital N and not notificationRequestItem. I would expect this to solve the issue because there is an Object serializer/deserializer in the models. See line 28 of the notification Request.

Here is an example constructing the NotificationRequest using a json with capital N

@tomdebuyser
Copy link
Author

tomdebuyser commented Jun 15, 2020

@KadoBOT here you see (a part of) my function where I try to map a notification to an internal payment state.

private mapNotificationToPaymentStatus(
        notification: NotificationRequestItem,
    ): PaymentStatus {
        switch (notification.eventCode) {
            case NotificationEventCode.AUTHORISATION: {
                if (notification.success === NotificationSuccess.True)
                    return PaymentStatus.FinishedByUser;
                return PaymentStatus.Refused;
            }
            ...
        }
    }

As you can see I want to compare eventCode and success, where I have to use the typings to make my code compile. How can I write this differently without casting these things to any (which I handle in the enums I created myself)?

@KadoBOT
Copy link
Contributor

KadoBOT commented Jun 15, 2020

There are two ways. The one I suggest is comparing to the String representation as currently, the library does not provide a default way to compare against an enum;

The other way is a bit hackish but you could try using the function for the eventCode enum which is generated from the compiled Object serializer, here

Keep in mind that the last one, is a generated file and we can't provide support for it if you decided to use it. And it might change anytime.

That's why I suggest using the String comparison until we provide the types.

@tomdebuyser
Copy link
Author

Ok. Looking forward to the types lib!

@KadoBOT
Copy link
Contributor

KadoBOT commented Jun 15, 2020

I'll keep this issue open until we release the types. Thanks @tomdebuyser !

@stale
Copy link

stale bot commented Aug 14, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Aug 14, 2020
@stale stale bot closed this as completed Aug 21, 2020
@hobadams
Copy link

hobadams commented Jul 29, 2021

@KadoBOT - any update on this?

We also have some other types that it would be nice to be exported so we can import them directly from @adyen/api-library

import Checkout from '@adyen/api-library/lib/src/services/checkout';
import { LineItem } from '@adyen/api-library/lib/src/typings/checkout/lineItem';
import { PaymentMethodsRequest } from '@adyen/api-library/lib/src/typings/checkout/paymentMethodsRequest';
import { PaymentResponse } from '@adyen/api-library/lib/src/typings/checkout/paymentResponse';

It would be ideal to have a type for the entire web hook body

@wboereboom wboereboom reopened this Jul 29, 2021
@stale stale bot removed the wontfix This will not be worked on label Jul 29, 2021
@wboereboom wboereboom added the Feature New feature or request label Jul 29, 2021
@wboereboom
Copy link
Contributor

Hi @hobadams,

Thanks for bumping this thread with your suggestion.
I'm going to mark this as a feature and will start working on making these types available directly from the library as they should be.

Kind regards,
Wouter
Adyen

This was referenced Sep 9, 2021
@stale
Copy link

stale bot commented Sep 28, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Sep 28, 2021
@stale stale bot closed this as completed Oct 11, 2021
@m3co-code
Copy link

The issue should be opened again or was the enum type problem fixed?

It's actually quite problematic as for each and every enum value we have to cast the type to any and then to a string to make our programs works :-/

@wboereboom
Copy link
Contributor

Hi @mjeri,

Unfortunately we do not have a fix for your issue at this time.
Could you make a new issue regarding this problem? As this thread consists of quite a few different topics right now.
We'll use this new issue to keep track of the problem until we are able to resolve it.
For now a converter function could act as an intermediate to keep the amount of duplicated code to a minimum.

Kind regards,
Wouter
Adyen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Feature New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

5 participants