Skip to content

Client::Add#12

Closed
bedrock-adam wants to merge 20 commits intomasterfrom
client/add_module
Closed

Client::Add#12
bedrock-adam wants to merge 20 commits intomasterfrom
client/add_module

Conversation

@bedrock-adam
Copy link
Contributor

@bedrock-adam bedrock-adam commented Apr 28, 2020

Motivation (Why)

build a well documented “low level” api library as a gem with the intent to open source it focused on the endpoints we know /suspect will be used by future xpm integration

https://ignitionapp.slack.com/archives/C0125F6R353/p1587950291147900

when a pi user is created, create a wfm / xpm client

Implementation (How)

  • Use Nokogiri::Xml::Builder to build a minimal client (with name) document
  • Post this document to endpoint using our new Connection.post interface
  • Return a minimal client as defined off the Get endpoint, in accordance with the docs

Note: I've moved a few things around in this PR because models/client.rb wasn't making much sense. client passed in to .add has a different structure from the client response (same as the .get response). Trying to merge these differences into one model will break conformity.

@bedrock-adam bedrock-adam changed the title refactor Client.add => Client::Add Client::Add Apr 28, 2020
@bedrock-adam bedrock-adam marked this pull request as ready for review April 28, 2020 02:16

require "nokogiri"

require "xpm_ruby/version"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this line is in twice? I moved it down earlier because I wanted to put them in alphabetical order. ;) My bad, you might not have noticed...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sorry merge conflict I'll fix that up.

:signed_tax_authority, :tax_agent, :agency_status, :return_type,
:prepare_activity_statement, :prepare_tax_return

def initialize(name:, email: nil,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to pass in the XML hash instead of each attribute?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah wait this is a different client object?

@bedrock-adam bedrock-adam mentioned this pull request Apr 28, 2020
@bedrock-adam
Copy link
Contributor Author

Closing in favour of Client.add

@bedrock-adam bedrock-adam deleted the client/add_module branch April 28, 2020 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants