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

Spec dispute documents #571

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Spec dispute documents #571

wants to merge 3 commits into from

Conversation

remear
Copy link
Contributor

@remear remear commented Apr 16, 2014

Spec-ing out dispute documents prior to implementation.

@steveklabnik Could you look over this first attempt.

@steveklabnik
Copy link
Contributor

Seems reasonable to me!

@tarunc
Copy link

tarunc commented Apr 17, 2014

Maybe we should add file_size to dispute document?

One more thing-- should file_type have "other" in the enum if it is not within one of those categories? Or should we even limit it because there are lot of file types that fall under the category of document/image (e.g. docx, odt, txt, rtf, png, tif...)

@remear
Copy link
Contributor Author

remear commented Apr 17, 2014

I thought about this earlier today but I omitted it because I couldn't think of a reason we really needed to have it. It certainly doesn't really hurt anything to provide though.

On Apr 16, 2014, at 10:29 PM, Tarun Chaudhry [email protected] wrote:

Maybe we should add file_size?


Reply to this email directly or view it on GitHub.

step 'I have debited a card that will initiate a dispute with submitted documents'
@dispute_id = nil
count = 60
while @dispute_id.nil? and count > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

@remear what do you think about removing this polling shit and tasking someone to fix the dispute latency? this is a cancer across all our specs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

anigif_enhanced-buzz-30309-1388524076-1

Completely agree. This polling stuff horrible.

@mjallday
Copy link
Contributor

providing a file_size attribute would mean that we could display the size of the file in the UI without actually downloading the file.

e.g. file_name.pdf 1.2MB

i think that is a good idea personally.

@remear
Copy link
Contributor Author

remear commented Apr 17, 2014

@tarunc Presently we're restricting file types to only: pdf, doc, jpg

I'm going to clarify with @andrewnossiter about the doc vs docx requirement. As deduced from prior conversations, the processors only allow these 3 document types so no other file_type.


Scenario: Retrieve a dispute document
Given I have a dispute with submitted documents
When I GET to /dispute_documents/:dispute_document_id
Copy link
Contributor

Choose a reason for hiding this comment

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

@steveklabnik do you like dispute_documents vs evidence? What's your reasoning for choosing one over the other?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pluralization of evidence was what steered me away from using it as a resource name.

Copy link
Contributor

Choose a reason for hiding this comment

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

"evidence" can be evidence of anything, "dispute document" is extremely unambiguous.

I don't feel particularly strongly about it, though.

@mahmoudimus
Copy link
Contributor

This looks gr8! Great job @remear

@remear
Copy link
Contributor Author

remear commented Apr 17, 2014

Added file_size and changed file_type to mime_type with proposed allowed mime types.

@remear
Copy link
Contributor Author

remear commented Apr 18, 2014

@balanced/engineers Any other adjustments?

"application/pdf",
"application/msword",
"application/vnd.openxmlformats-officedocument.wordprocessingml.document",
"image/jpeg"
Copy link
Contributor

Choose a reason for hiding this comment

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

pngs too? my mac creates a png everytime i take a screenshot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In #563 (comment) it was stated that the only image format AMEX accepts is jpeg.

@remear
Copy link
Contributor Author

remear commented Apr 23, 2014

Dispute should have a boolean dispute_documentation_submitted field to signify if documents have been submitted to the processor. Since documentation may only be submitted one time, this will provide a simple check.

ping @balanced/engineers Thoughts?

@mahmoudimus
Copy link
Contributor

Would probably have more context, disputed_documentation_submitted_at

@remear
Copy link
Contributor Author

remear commented Apr 23, 2014

Yes. That's better.

@mahmoudimus
Copy link
Contributor

Should we create a github team called "spec-patrol" instead of engineers so it's not as noisy?

@mjallday
Copy link
Contributor

disputed_documentation_submitted_at -> documentation_submitted_at. unless the documentation has been disputed.

"properties": {
"links": {
"type": "object",
"properties": {}
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs a dispute_document.dispute reference

@fangpenlin
Copy link
Contributor

@mahmoudimus @mjallday I didn't notice we had this spec for dispute document uploading, it looks like this is the spec for the API living in balanced service. But for now, justitia is an independent service. The json schema of justitia can be found here:

https://github.com/balanced/justitia/tree/master/tests/fixtures/schemas

So, should we mount corresponding URL endpoints in balanced API to justitia.balancedpayments.com? And should I modify justitia to make it works like this spec described?

@mjallday
Copy link
Contributor

that's a good question @victorlin

@mahmoudimus how do you envision api and disputes services interacting?

is this disputes service mounted under api.balancedpayments.com/some-endpoint or does it run as a standalone service at disputes.balancedpayments.com?

the spec specifies a relative path for the dispute document - https://github.com/balanced/balanced-api/pull/571/files#diff-d8ae36f8a33d67242bf4cc1510101631R21 - i think we should tweak the pull-request and make those uris absolute instead.

@remear remear mentioned this pull request Jan 7, 2015
@kyungmin
Copy link

ping @mahmoudimus

how do you envision api and disputes services interacting?

What else needs to be done to complete the API spec?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants