Skip to content

Properly check value using symbols#12

Merged
tjwallace merged 2 commits into
masterfrom
fix_inspection_methods
Sep 10, 2021
Merged

Properly check value using symbols#12
tjwallace merged 2 commits into
masterfrom
fix_inspection_methods

Conversation

@tjwallace

@tjwallace tjwallace commented Aug 16, 2021 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Enum attributes are returned as symbols.

Slack conversation

BankAccountService development (main) 6> pf.status
:pending
BankAccountService development (main) 7> pf.status_pending?
false

Why are there no specs?

I don't know...

@tjwallace tjwallace requested review from dpep and tilo August 16, 2021 23:34
@mattmccormick

mattmccormick commented Aug 16, 2021

Copy link
Copy Markdown

Added @jaredsmithse as a reviewer as he did a deep dive into this a month or so back and presented his findings to the team. I was running into a bug that ended up being caused by sql_enum.

jaredsmithse
jaredsmithse approved these changes Sep 10, 2021
View reviewed changes

@jaredsmithse jaredsmithse left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm guessing this was tested out manually since there were no specs?

@tjwallace

tjwallace commented Sep 10, 2021

Copy link
Copy Markdown
Collaborator Author

Testing with bank-account-service:

BankAccountService PRODUCTION RO (main):001:0> pf = PayFriend.last
#<PayFriends::ToMember:0x00007f976c02f980> ...
BankAccountService PRODUCTION RO (main):002:0> pf.status
:failed
BankAccountService PRODUCTION RO (main):003:0> pf.status_failed?
true
BankAccountService PRODUCTION RO (main):004:0> pf.status_pending?
false

@tjwallace tjwallace merged commit 4f97628 into master Sep 10, 2021
@tjwallace tjwallace deleted the fix_inspection_methods branch September 10, 2021 21:52
@tjwallace tjwallace mentioned this pull request Sep 10, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tilo tilo Awaiting requested review from tilo

@dpep dpep Awaiting requested review from dpep

1 more reviewer

@jaredsmithse jaredsmithse jaredsmithse approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tjwallace @mattmccormick @jaredsmithse