search

cclrobotics / ARTBot

Project to allow submissions of agar art and conversion of those submissions to bot-readable
MIT License
8 stars 5 forks source link

[refactor] Major app refactor #38

Closed brcb20 closed 4 years ago

brcb20 commented 4 years ago

BUG: in robot script (see comments in file)

brcb20 commented 4 years ago

@tsdobbs this is now ready to merge, please double check functionality and content of error messages and emails.

tsdobbs commented 4 years ago

Got it. Will test and deploy this week.

On Mon, Nov 25, 2019, 9:40 AM Benjamin B notifications@github.com wrote:

@tsdobbs https://github.com/tsdobbs this is now ready to merge

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cclrobotics/ARTBot/pull/38?email_source=notifications&email_token=AEB3IRSFVGUP6S6RJRMW7R3QVQEYBA5CNFSM4JJ4WPPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFDGVTA#issuecomment-558262988, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEB3IRX3MQVNMNWUXCANJ6DQVQEYBANCNFSM4JJ4WPPA .

tsdobbs commented 4 years ago

On email send failure, user is still shown a success message and told that a confirmation email with be sent even though the email is not sent. Failed send is logged correctly, but we should probably communicate to user in some way, or raise a flag for admins

tsdobbs commented 4 years ago

Confirmation email looks great. Issues:

brcb20 commented 4 years ago

On email send failure, user is still shown a success message and told that a confirmation email with be sent even though the email is not sent. Failed send is logged correctly, but we should probably communicate to user in some way, or raise a flag for admins

Communicating to the user is not possible with the current setup. The emails are sent async and we do not wait to see if the email succeeded. Raising a flag for an admin could be done by setting up log handling with email (to admin). Although handling an email failure by sending another email might not be the best course of action. For now, maybe we can just check the db regularly, and understand under what conditions failures occur.

tsdobbs commented 4 years ago

Seems like an acceptable solution. We'll need to manually force an email send when we discover the problem, I assume, then. Or we can assume guess whether the email is legit and confirm the artpiece manually.

(FYI: Code is merged to master)

On Fri, Dec 13, 2019 at 6:40 PM Benjamin B notifications@github.com wrote:

On email send failure, user is still shown a success message and told that a confirmation email with be sent even though the email is not sent. Failed send is logged correctly, but we should probably communicate to user in some way, or raise a flag for admins

Communicating to the user is not possible with the current setup. The emails are sent async and we do not wait to see if the email succeeded. Raising a flag for an admin could be done by setting up log handling with email (to admin). Although handling an email failure by sending another email might not be the best course of action. For now, maybe we can just check the db regularly, and understand under what conditions failures occur.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/cclrobotics/ARTBot/pull/38?email_source=notifications&email_token=AEB3IRXGD2KZI4VIKFYMBRDQYRBQLA5CNFSM4JJ4WPPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG3X52Q#issuecomment-565673706, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEB3IRVM6PER6WGMQZHQ4NLQYRBQLANCNFSM4JJ4WPPA .