Skip to content

Conversation

@prathamesh-sonpatki
Copy link
Member

@rails-bot rails-bot bot added the actionpack label Mar 12, 2019
@kaspth
Copy link
Contributor

kaspth commented Mar 12, 2019

Did you already have Active Storage enabled in the app you tested with? I’m not sure the error message would hit on Action Mailbox first?

@prathamesh-sonpatki
Copy link
Member Author

@kaspth No I didn't have active storage enabled. I was testing the conductor page and clicked on the Action Mailbox Inbound emails link: https://monosnap.com/file/RKUdXiXgy5fY4oAR6fGb6ksle33yRG

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably needs to be outdented like the line above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated!

@kaspth
Copy link
Contributor

kaspth commented Mar 12, 2019

Cool! I just wanted to make sure we didn't say that "Active Storage must be installed" for Action Mailbox being missing (yes, it's a dependency but the error should focus on Action Mailbox).

@kaspth
Copy link
Contributor

kaspth commented Mar 12, 2019

If you want to convert this later to use #34788, do feel free to translate them! Perhaps as a PR to that branch?

cc @gsamokovarov

…does not exist

- This is similar to the work done in rails#31534
@prathamesh-sonpatki
Copy link
Member Author

If you want to convert this to use #34788, do feel free to translate them! Perhaps as a PR to that branch?

cc @gsamokovarov

@kaspth Sure, I will do that. Can I do that once this PR is merged or are you suggesting to convert this PR itself?

@kaspth kaspth merged commit 4948663 into rails:master Mar 12, 2019
@kaspth
Copy link
Contributor

kaspth commented Mar 12, 2019

I meant after, but it wasn't clear in the message. But that's fixed now that I merged 😄

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.

3 participants