Deprecated enums still being used

[mittal-umang]

Fixes #931

[CLAassistant]

All committers have signed the CLA.

[mwwoda]

Hi @mittal-umang

Thanks for the contribution! This is something that was overlooked when deprecating the old BoxEvent.Type enum. Right now your PR is actually breaking existing API. We would prefer to have a similar methods with same logic that takes new BoxEvent.EventType and deprecate the old ones. That way we can give existing customers time to migrate to the new methods. I would gladly accept this PR if you can introduce such change.

Thanks,
Mateusz

[mittal-umang]

Hi Mateusz, Thank you for the quick response. I updated the code locally but i'm having issues resolving the test cases. I deprecated the functions with Types and created an update copy of them with EventTypes. Which is currently breaking the test case in EventLogIT.java. The current test cases are missing last arguments either EventTypes or Types. I'm happy to resolved this issue if you could help me with this. Unless i need to create new methods with different method name for arguments with EventType i could do that. and subsequently add a function call in the test scripts. Regards, Umang Mittal +1 (404) 952-5883

…

On Thu, Oct 28, 2021 at 9:46 AM Mateusz Woda ***@***.***> wrote: Hi @mittal-umang <https://github.com/mittal-umang> Thanks for the contribution! This is something that was overlooked when deprecating the old BoxEvent.Type enum. Right now your PR is actually breaking existing API. We would prefer to have a similar methods with same logic that takes new BoxEvent.EventType and deprecate the old ones. That way we can give existing customers time to migrate to the new methods. I would gladly accept this PR if you can introduce such change. Thanks, Mateusz — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#932 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALEXXUDTIN4KMXX34OOMUBDUJFO53ANCNFSM5G3EIYDQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[mittal-umang]

Hi Mateusz, The other option is i extend the current EventLog.java and update all the methods with in. But i would need help with the name since i'm not sure what would be the java standard for this. Regards, Umang Mittal +1 (404) 952-5883 On Thu, Oct 28, 2021 at 11:09 AM Umang Mittal ***@***.***> wrote:

…

Hi Mateusz, Thank you for the quick response. I updated the code locally but i'm having issues resolving the test cases. I deprecated the functions with Types and created an update copy of them with EventTypes. Which is currently breaking the test case in EventLogIT.java. The current test cases are missing last arguments either EventTypes or Types. I'm happy to resolved this issue if you could help me with this. Unless i need to create new methods with different method name for arguments with EventType i could do that. and subsequently add a function call in the test scripts. Regards, Umang Mittal +1 (404) 952-5883 On Thu, Oct 28, 2021 at 9:46 AM Mateusz Woda ***@***.***> wrote: > Hi @mittal-umang <https://github.com/mittal-umang> > > Thanks for the contribution! This is something that was overlooked when > deprecating the old BoxEvent.Type enum. Right now your PR is actually > breaking existing API. We would prefer to have a similar methods with same > logic that takes new BoxEvent.EventType and deprecate the old ones. That > way we can give existing customers time to migrate to the new methods. I > would gladly accept this PR if you can introduce such change. > > Thanks, > Mateusz > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#932 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/ALEXXUDTIN4KMXX34OOMUBDUJFO53ANCNFSM5G3EIYDQ> > . > Triage notifications on the go with GitHub Mobile for iOS > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> > or Android > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. > >

[antusus]

Hi @mittal-umang,

Sorry for late response. I can see the problem with duplicating methods having varargs arguments.
I'm a big fan of request objects. It allow us to define required and optional parameters. And I would go this way in this scenario. In fact I'm currently working on this method as I want to prepare SDK to work with new stream of events. Here is the method definition:

public static EventLog getEnterpriseEvents(BoxAPIConnection api, EnterpriseEventsRequest request)

Where EnterpriseEventsRequest will contain all the parameters like after, before, limit, etc. This new method will be using new enum values. Example of usage:

EnterpriseEventsRequest request = new EnterpriseEventsRequest()
    .after(after)
    .before(before)
    .limit(limit)
    .position(position)
    .types(eventTypes);
EventLog.getEnterpriseEvents(api, request);

I will post PR link most likely next week.

[antusus]

Pull request that fixes the problem with deprecated enums being used: #934

[mittal-umang]

Hi Kamil, Does this PR fix the issue mentioned earlier. I'm currently not available to look at this completely. I would really appreciate if you could let me know if i still need to fix the code i pushed in the earlier PR? Regards, Umang Mittal +1 (404) 952-5883

…

On Tue, Nov 9, 2021 at 10:28 PM Kamil Berdychowski ***@***.***> wrote: Pull request that fixes the problem with deprecated enums being used: #934 <#934> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#932 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALEXXUD5ACGZTZS6AZCKWCTULFHNDANCNFSM5G3EIYDQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[antusus]

No you do not need to merge your PR. I went with completely different approach.
Old methods are now marked as deprecated and the new method uses only new BoxEvent.EventType.
This should solve #931 issue.

[antusus]

The issue should be resolved with release of 2.58.0