sreeise / graph-rs-sdk

Microsoft Graph API Client And Identity Platform Client in Rust
MIT License
114 stars 30 forks source link

Solutions and bookings #471

Closed buhaytza2005 closed 5 months ago

buhaytza2005 commented 6 months ago

I've added support for the Bookings API. It includes businesses, customers, appointments and custom questions. The PR includes a couple of examples too.

sreeise commented 6 months ago

Hey, thanks for doing this and apologies for not getting to this sooner. I have been very busy as of late.

I am currently in the middle of releasing the next version and once that is done I will take a look at this within the next couple of days.

buhaytza2005 commented 6 months ago

Hi @sreeise! Thanks for taking the time to review the PR.

Re the renaming of the methods: https://github.com/sreeise/graph-rs-sdk/pull/471/commits/f69985a224183e515054cddbb0a82725c450beca contains the changes but when I regenerated the code the methods were not replaced/renamed. Am I missing a trick or is the code incorrect?

I assumed that:

MethodMacroModifier::fn_name_and_path(
            "appointments", "/appointments/$count",
            GeneratedMacroType::FnName("get_appointments_count")
        ),

would be the correct implementation, am I wrong in my assumption? Or is there anything else that I would need to add?

Thanks, Mike

sreeise commented 6 months ago

Hi @sreeise! Thanks for taking the time to review the PR.

Re the renaming of the methods: f69985a contains the changes but when I regenerated the code the methods were not replaced/renamed. Am I missing a trick or is the code incorrect?

I assumed that:

MethodMacroModifier::fn_name_and_path(
          "appointments", "/appointments/$count",
          GeneratedMacroType::FnName("get_appointments_count")
      ),

would be the correct implementation, am I wrong in my assumption? Or is there anything else that I would need to add?

Thanks, Mike

You have the methods matching ResourceIdentity::Solutions but that won't work here. You'll have to add a match for each of the ResourceIdentity enums that match the api client struct where the method is implemented for each.

For instance ResourceIdentity::BookingBusinesses:

ResourceIdentity::BookingBusinesses => vec![
  MethodMacroModifier::fn_name_and_path(
    "booking_businesses", "/bookingBusinesses/$count",
    GeneratedMacroType::FnName("get_booking_businesses_count")
  ),
]
sreeise commented 5 months ago

This all looks correct. There is only one issue that this is actually my fault.

I forgot that in 2.0 the macro used for the struct definitions of clients was changed from resource_api_client to api_client but I guess I forgot to update 1 part of codegen where the name is formatted and written out:

https://github.com/sreeise/graph-rs-sdk/blob/b35f98e641ed17ad13f21c5729d1ffd8894394cc/graph-codegen/src/macros/macro_queue_writer.rs#L451

That is my fault I apologize.

You can do one of two things:

Either manually change resource_api_client calls in the solutions directory or update that codegen line above and rerun the codegen. Obviously that will have to get updated at some point if you just change your solutions directory but I don't want to require you to fix my mistake unless your willing and want to.

buhaytza2005 commented 5 months ago

Happy to fix, love helping.

I saw that after merge with 2.0 something went wrong, but thought I messed something up from my end 🤣 . I'll update the PR and resubmit sometime this weekend.

buhaytza2005 commented 5 months ago

All done but I had to manually replace resource_api_client due to #478 as I can't regenerate the code.

sreeise commented 5 months ago

All done but I had to manually replace resource_api_client due to #478 as I can't regenerate the code.

Ah gotcha. I'll take a look at that here soon and hopefully get it fixed. Thanks for doing this and good work. Its much appreciated.