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

Add optional support for a simple CAPTCHA. #844

Merged
merged 4 commits into from Oct 11, 2021
Merged

Conversation

almet
Copy link
Member

@almet almet commented Oct 10, 2021

Add support for a simple CAPTCHA. Alternate solution for #841.

@almet almet marked this pull request as draft October 10, 2021 14:35
@almet almet force-pushed the almet/simple-captcha branch 2 times, most recently from 4c7e594 to 0fc6ffa Compare October 10, 2021 14:49
@almet almet marked this pull request as ready for review October 10, 2021 14:54
@almet almet marked this pull request as draft October 10, 2021 14:57
@almet
Copy link
Member Author

almet commented Oct 10, 2021

Hey, I've tried adding these tests but it seems that they have some side effect on the other ones, which start to ask for a CAPTCHA even if they shouldn't.

I'm unsure as to when I'll be able to spend some more time on this, don't hesitate to take over if you feel like it.

class CaptchaTestCase(IhatemoneyTestCase):
    ENABLE_CAPTCHA = True
    def test_project_creation_with_captcha(self):
        with self.app.test_client() as c:
            res = c.post(
                "/create",
                data={
                    "name": "raclette party",
                    "id": "raclette",
                    "password": "party",
                    "contact_email": "raclette@notmyidea.org",
                    "default_currency": "USD",
                },
            )
            self.assertEqual(len(models.Project.query.all()), 0)

            res = c.post(
                "/create",
                data={
                    "name": "raclette party",
                    "id": "raclette",
                    "password": "party",
                    "contact_email": "raclette@notmyidea.org",
                    "default_currency": "USD",
                    "captcha": "nope"
                },
            )
            self.assertEqual(len(models.Project.query.all()), 0)

            res = c.post(
                "/create",
                data={
                    "name": "raclette party",
                    "id": "raclette",
                    "password": "party",
                    "contact_email": "raclette@notmyidea.org",
                    "default_currency": "USD",
                    "captcha": "euro"
                },
            )
            self.assertEqual(len(models.Project.query.all()), 1)

@@ -241,6 +242,50 @@ def test_invitation_email_failure(self):
)


class CaptchaTestCase(IhatemoneyTestCase):
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm curious why when put in this file it doesn't have side effects, because it had some when I put it in the budget_test.py file. Any idea?

Copy link
Member

Choose a reason for hiding this comment

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

In fact… no. I put it there just because I felt it was the right place, and it just worked.

@almet
Copy link
Member Author

almet commented Oct 10, 2021

Thanks for taking over @Glandos :-)

@almet almet marked this pull request as ready for review October 10, 2021 21:31
@almet
Copy link
Member Author

almet commented Oct 10, 2021

It's now ready to be merged.

@almet almet merged commit 2bcc41b into master Oct 11, 2021
@almet almet deleted the almet/simple-captcha branch October 11, 2021 15:39
TomRoussel pushed a commit to TomRoussel/ihatemoney that referenced this pull request Mar 2, 2024
* Add optional support for a simple CAPTCHA.
* formatting
* add test case
* Flake8

Co-authored-by: Glandos <bugs-github@antipoul.fr>
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.

None yet

2 participants