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

fix 'NoneType' object has no attribute 'utcoffset' #522

Closed
wants to merge 1 commit into from

Conversation

paulfab
Copy link

@paulfab paulfab commented Nov 14, 2021

(Describe the goal of this PR. Mention any related Issue numbers)

Category (place an x in each of the [ ])

  • slack_bolt.App and/or its core components
  • slack_bolt.async_app.AsyncApp and/or its core components
  • Document pages under /docs
  • Adapters in slack_bolt.adapter
  • [ x] Others

Requirements (place an x in each [ ])

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

  • [ x] I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • [x ] I've read and agree to the Code of Conduct.
  • I've run ./scripts/install_all_and_run_tests.sh after making the changes.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@seratch seratch added area:examples issues related to example or sample code bug Something isn't working labels Nov 14, 2021
@seratch seratch self-assigned this Nov 14, 2021
@seratch
Copy link
Member

seratch commented Nov 14, 2021

Hi @paulfab, thanks for taking the time to fix this error! Would you mind signing the CLA (please check the above bot message)? Without having it, we are unable to merge your changes in this repository.

@seratch seratch added this to the 1.11.0 milestone Nov 14, 2021
@@ -58,7 +58,7 @@ def save_bot(self, bot: Bot):
b = bot.to_dict()
if is_naive(b["installed_at"]):
b["installed_at"] = make_aware(b["installed_at"])
if "bot_token_expires_at" in b is not None and is_naive(
Copy link
Member

Choose a reason for hiding this comment

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

Obviously, this part was wrong.

@@ -33,9 +33,9 @@ def save(self, installation: Installation):
i = installation.to_dict()
if is_naive(i["installed_at"]):
i["installed_at"] = make_aware(i["installed_at"])
if "bot_token_expires_at" in i and is_naive(i["bot_token_expires_at"]):
if i.get("bot_token_expires_at") and is_naive(i["bot_token_expires_at"]):
Copy link
Member

Choose a reason for hiding this comment

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

Can you change this way for consistency?

Suggested change
if i.get("bot_token_expires_at") and is_naive(i["bot_token_expires_at"]):
if i.get("bot_token_expires_at") is not None and is_naive(i["bot_token_expires_at"]):

i["bot_token_expires_at"] = make_aware(i["bot_token_expires_at"])
if "user_token_expires_at" in i and is_naive(i["user_token_expires_at"]):
if i.get("user_token_expires_at") and is_naive(i["user_token_expires_at"]):
Copy link
Member

Choose a reason for hiding this comment

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

same as above

Suggested change
if i.get("user_token_expires_at") and is_naive(i["user_token_expires_at"]):
if i.get("user_token_expires_at") is not None and is_naive(i["user_token_expires_at"]):

@codecov
Copy link

codecov bot commented Nov 15, 2021

Codecov Report

Merging #522 (8275426) into main (1b5f4a1) will not change coverage.
The diff coverage is n/a.

❗ Current head 8275426 differs from pull request most recent head b058b01. Consider uploading reports for the commit b058b01 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main     #522   +/-   ##
=======================================
  Coverage   91.29%   91.29%           
=======================================
  Files         169      169           
  Lines        5606     5606           
=======================================
  Hits         5118     5118           
  Misses        488      488           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b5f4a1...b058b01. Read the comment docs.

@seratch
Copy link
Member

seratch commented Nov 15, 2021

@paulfab Thanks for updating this pull request. Would you mind signing the CLA (please check the above bot message)? Without having it, we are unable to merge your changes in this repository.

@paulfab
Copy link
Author

paulfab commented Nov 15, 2021

Hey @seratch you are so fast I didnt have the time to comment ^^. Thanks for reviewing the PR, I'm quite reluctant to sign the CLA. You can take ownership of my modest changes and merge it ?

@seratch
Copy link
Member

seratch commented Nov 15, 2021

@paulfab Okay, in the case, I can come up with my own pull request instead. Thanks a lot for taking the time to report this!

@seratch
Copy link
Member

seratch commented Nov 15, 2021

Let me close this pull request in favor of #523

@seratch seratch closed this Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:examples issues related to example or sample code bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants