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

Refactor default inquiry message on mobile to use MP #2127

Open
jonallured opened this issue Feb 1, 2018 · 0 comments
Open

Refactor default inquiry message on mobile to use MP #2127

jonallured opened this issue Feb 1, 2018 · 0 comments

Comments

@jonallured
Copy link
Member

While working on improving the default inquiry message for on loan works, I discovered that there are two places where these messages come from. The first I found was here in force, so I made this PR:

#2117

But then when @anandaroop and I went to go smoke test it, we realized that it wasn't actually fixing the problem. That lead us to discover that this default message was actually coming from MP and that lead me to make this PR:

artsy/metaphysics#921

Once that was merged and deployed, we saw that the default message was working as expected. I assumed the next step would be to nuke that dead code in force, but @mzikherman helped me see that it's actually used on the mobile side of things, so we ended up merging #2117 so that the default would be correct there as well.

This was the pragmatic thing to do, but what would have been even better would have been to refactor the mobile side of thing to use MP as well so that there is only one code path for computing these messages. I'm opening this ticket to highlight this need.

/cc @katarinabatina

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

No branches or pull requests

1 participant