Skip to content

Conversation

@zealot128
Copy link

@zealot128 zealot128 commented Sep 13, 2021

Hey, thankfully i've discovered your repository which was a great start to integrate our App with MS365. I've added tons of actions, but refactored it in the meantime. I don't expect this to be merged (ever/anytime soon), as there are also no tests atm. But nevertheless wanted to make sure you or other user can see our changes.

  • Refactored most to support a common Chainable Syntax by using a common base class
  • Unified (at least all the classes I've touched/used) the call syntax:
    • Never need to supply the path from outside, but rather determine the path when given a user_id, otherwise use the me/ variants.
  • ChainableAction as Base class , used it for all my new actions and for users (The other ones, mails todos etc. I didn't touch at all, because our app does not need it and has not the scope to request it anyways)
    • Suppots Auto Paginate All by using .all instead of .get. internally uses .get to request each page until there are no more pages left.
    • unified syntax for .select .filter .top .skip etc. without having to implement it by each action again.

New Api calls:

  • rooms list
  • OnlineMeeting CRUD
  • calendards
  • calendarGroups
  • calendarGetSchedule (verrrry useful -> Get free/busy schedule of resources, coworkrs)

Rubocop:

  • I've disabled ParameterLists and MethodLength calls. They had some preconfigured values which seemed to be arbitrary anyways and just based on the existing code base. ParameterLists does not make sense for the Gem IMO, because stuff like "Create Event" will have a ton of arguments. If we keep the API surface as is, then having all the arguments as keyword args is convient. The only way to fulfill this cop would be to refactor all api calls to not take keyword args, but a kind of Input Type / Form object, that holds all the parameters. Method Lengths would only lead to spread out function or useless Ruby Golfing in my opinion. I don't find the offending methods (CalenderCreate etc.) too long, they have only like 3-4 real code statements, they are just long because I map the kwargs to the expected json for MS Graph.

Missing:

  • tests, haven't had time to look into unit tests, as our App that uses this Gem has integration tests test test to the live Microsoft 365 developer sandbox.

@grantspeelman
Copy link
Contributor

thanks for sharing this @zealot128 👍🏽

I recently discovered that microsoft graph is actually an odata service. So playing with the idea of instead making this a thin wrapper around something like https://github.com/getoutreach/frodo, which already supports chaining.

If that fails we can look into incorporating these changes some how.

thanks again

@grantspeelman grantspeelman added the enhancement New feature or request label Sep 14, 2021
@lauer
Copy link
Contributor

lauer commented Jun 14, 2022

@grantspeelman how did it do? The project seems kind of dead? Just look at this PR which I also would need for my project.
getoutreach/frodo#52

@grantspeelman
Copy link
Contributor

grantspeelman commented Jun 14, 2022

It's still only a idea, have not done anything more beyond the thought 😃

@lauer
Copy link
Contributor

lauer commented Jun 24, 2022

I tested with https://github.com/shaneshort/frodata which what the first I found which did not conflict with the faraday version.
However, I get weird error, when I am trying to access Microsoft Graph API users.

I cannot fetch complex_types (service.complex_types) and therefore not fetching a single user with service['users'].first
This gives me Unknown property type: graph.employeeOrgData

So please let me know if you have found a solution to this?
I have also tried this version, but same problem: https://github.com/seesaw/frodata/tree/s%26m

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants