Skip to content

Conversation

@stefan-cooper
Copy link
Contributor

@stefan-cooper stefan-cooper commented Jun 29, 2022

Contributes to: https://github.com/KoalaBotUK/KoalaBotFrontend/issues/439
close #391
close #392
close #390

Signed-off-by: Stefan Cooper stefan.cooper27@gmail.com
Signed-off-by: Jack Draper drapj002@gmail.com

Summary

Delivering this separately to the API work as there were quite a few big changes needed within the logic of RFR to make this possible.

Checklist

  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

  • Have you tested the changes? (pytest & dpytest)
  • Have you followed PEP-8 for naming and styling?
  • Has your code been properly documented with RestructuredText docstrings?
  • Have you added your changes to CHANGELOG.md under the [Unreleased] heading?
  • If your code added new bot commands, have you updated documentation.json?

  • All of this code is your own, or follows the author repo's licence.

stefan-cooper and others added 2 commits June 30, 2022 00:43
Contributes to: KoalaBotUK/KoalaBotFrontend#439

Signed-off-by: Stefan Cooper <stefan.cooper27@gmail.com>
@codecov
Copy link

codecov bot commented Jun 29, 2022

Codecov Report

Merging #313 (c8d4a8b) into master (6e4eec3) will increase coverage by 0.08%.
The diff coverage is 81.28%.

@@            Coverage Diff             @@
##           master     #313      +/-   ##
==========================================
+ Coverage   87.30%   87.39%   +0.08%     
==========================================
  Files         115      119       +4     
  Lines        8399     8750     +351     
==========================================
+ Hits         7333     7647     +314     
- Misses       1066     1103      +37     
Flag Coverage Δ
unittests 87.39% <81.28%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
koala/cogs/react_for_role/cog.py 49.01% <38.59%> (-0.08%) ⬇️
koala/cogs/react_for_role/__init__.py 60.00% <60.00%> (-40.00%) ⬇️
koala/cogs/react_for_role/core.py 68.32% <68.32%> (ø)
koala/cogs/react_for_role/db.py 83.33% <83.14%> (-12.42%) ⬇️
koala/cogs/react_for_role/api.py 91.66% <91.66%> (ø)
tests/cogs/react_for_role/test_cog.py 84.95% <91.66%> (+0.50%) ⬆️
koala/cogs/react_for_role/dto.py 100.00% <100.00%> (ø)
koala/rest/api.py 92.18% <100.00%> (+6.73%) ⬆️
tests/cogs/base/test_api.py 100.00% <100.00%> (ø)
tests/cogs/react_for_role/test_api.py 100.00% <100.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Contributes to: KoalaBotUK/KoalaBotFrontend#439

Signed-off-by: Stefan Cooper <stefan.cooper27@gmail.com>
Comment on lines +186 to +187
# TODO - Get this working, for some reason we get 403 currently
# await core.setup_rfr_reaction_permissions(ctx.guild, channel, self.bot)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JayDwee This is the only outstanding piece I can't quite get working. When the permission setups is used inside core it gets a 403.

@lgtm-com
Copy link

lgtm-com bot commented Jun 29, 2022

This pull request introduces 5 alerts when merging 4a19ef6 into cc08f61 - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 2 for Module is imported with 'import' and 'import from'
  • 1 for Module is imported more than once

Copy link
Member

@JayDwee JayDwee left a comment

Choose a reason for hiding this comment

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

db2.py needs renaming/merging with db.py
Also some additional tests need to be added to increase coverage
other than that, looks good. I'll take a look at your issue later this week hopefully

@VottonDev VottonDev self-assigned this Oct 31, 2022
@lgtm-com
Copy link

lgtm-com bot commented Oct 31, 2022

This pull request introduces 5 alerts when merging 5bb8a15 into 8713ece - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 2 for Module is imported with 'import' and 'import from'
  • 1 for Module is imported more than once

@lgtm-com
Copy link

lgtm-com bot commented Oct 31, 2022

This pull request introduces 2 alerts when merging b3b90d0 into 8713ece - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for 'import *' may pollute namespace

@JayDwee JayDwee changed the title refactor: move re-usable rfr logic out of cog to core ReactForRole API Mar 28, 2023
@JayDwee
Copy link
Member

JayDwee commented Mar 28, 2023

Waiting for CraftSpider/dpytest#102 to be pushed to dpytest. after which this PR should be good to merge after reverting 7c73d93

@JayDwee JayDwee merged commit 44492c8 into master Apr 4, 2023
@JayDwee JayDwee deleted the feat/rfr-api-cog branch April 4, 2023 20:14
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

Successfully merging this pull request may close these issues.

RFR: Complete API implementation & PR React For Role API documentation React For Role API

5 participants