#8013 New Users should not be displayed in /u/wiki/home until email is verified

v1.4.0
closed
nobody
None
User
Heith Seewald
2015-12-17
2015-10-30
Heith Seewald
No

Right now new users are displayed in '/u/wiki/home' before email validation. The link for that user will just 404 until email is verified.

This shouldn't apply if auth.require_email_addr is set to false in the ini.

Related

Git: 739aea00
Git: c544a7ac

Discussion

  • Pranav Sharma - 2015-12-02

    What is the wiki for ? '/u/wiki/home' is project wiki or what? And which users are currently shown there?

     

    • Dave Brondsema

      Dave Brondsema - 2015-12-02

      /u/ is the Users neighborhood which is created automatically and contains the personal projects for everyone. The wiki is a neighborhood-level tool which is installed by default and uses the [[projects show_total=yes]] macro. That macro is a general one that works in any neighborhood and lists all the projects in the neighborhood.

      So I guess we would need some logic in that macro to check if it is a user-project and if the user is pending. Or probably better: not create the user-project at all when a user is just pending. Create it after they are activated.

       

      • Pranav Sharma - 2015-12-05

        Would taking away the permission of creating projects from a non-verified user would be enough?

         

        • Dave Brondsema

          Dave Brondsema - 2015-12-05

          The user-project is created automatically. E.g. the /u/pranav/ project was created for you when you created your account. I think we need to find where that is happening and do it later in the process, so it only is created when the account is activated.

           

  • Pranav Sharma - 2015-12-09
    • status: open --> in-progress
     

  • Heith Seewald - 2015-12-09

    Hey Pranav -- your merge request is looking good.

    There is just a couple of things:

    1. if auth.require_email_addr = false in the settings, then a user project never gets created. You can probably fix it with something like: make_project=(not require_email)
    2. There are a few minor pep8 violations.
     

  • Heith Seewald - 2015-12-11

    Hey Pranav -- nice work. This appears to work well.

    The pep8 issues are minor -- but there are a few tools to make checking for violations.

    I recommend setting up your editor to display pep8 issues in real time -- but you can also use flake8 (which supports git diffs).

    for example:

    git diff origin/master | flake8 --diff

    gives the following output:

    ./Allura/allura/controllers/auth.py:238:40: E231 missing whitespace after ','
    ./Allura/allura/controllers/auth.py:239:120: E501 line too long (144 > 119 characters)
    ./Allura/allura/controllers/auth.py:275:47: E711 comparison to None should be 'if cond is None:'
    ./Allura/allura/controllers/auth.py:282:32: E128 continuation line under-indented for visual indent

     

  • Heith Seewald - 2015-12-15

    Nice work man :)

    Could you also add an assertion (or two) in tests.functional.test_auth.TestAuth#test_create_account_require_email?

    I should have mentioned that in a previous comment -- sorry about that.

     

    • Pranav Sharma - 2015-12-16

      What should the functional tests confirm exactly?

       

  • Heith Seewald - 2015-12-16

    There are two assertions currently in test_create_account_require_email.
    After each assertion, we could just put a simple check confirming the existence of a 'user' project.

        def test_create_account_require_email(self):

            #...

            assert not user.pending
            assert_equal(M.Project.query.find({'name': 'u/aaa'}).count(), 1)

            #...

            assert user.pending
            # Pending users should not have a user project
            assert_equal(M.Project.query.find({'name': 'u/bbb'}).count(), 0)
     

    • Pranav Sharma - 2015-12-16

      https://forge-allura.apache.org/u/pranav/allura/ci/8530054c9b3716e186ec290bc9334f96c5a612e5/

      Thanks. Also, where could I learn more about the assertions (basically syntax). Also, which modifications really need assertions?

      I also tried this - 

      assert (user.private_project() == None)
assert not (user.private_project() == None)

      at right places.
      but it failed. Why? Do you use TDD approach to code?

       

      Last edit: Pranav Sharma 2015-12-16

  • Heith Seewald - 2015-12-16

    Hey Pranav -- good idea using private_project().

    Try something like: 

        #...

    assert not user.pending
    assert_is_not_none(user.private_project())

    #...

     assert user.pending
    assert_is_none(user.private_project())

    We're using nose for testing in Allura. Nose allows for a mixture of assertion styles. You can use the built-in python assert, or any one of the functions from nose.tools (such as assert_equals). The main advantage that something like assert_equals has over a standard assertion is that it results in a more readable error message when a failure occurs.

    As to why it didn’t work when you tried it -- I’m not sure. Did you have any error messages?
    This should work:

        #...

    assert not user.pending
    assert user.private_project()

    #...

     assert user.pending
     assert user.private_project() is None

    If you want to checkout some of the other built-in nose assertions:

    import nose
dir(nose.tools) # Or use ipython
     

  • Heith Seewald - 2015-12-17
    • status: in-progress --> closed
    • Reviewer: Heith Seewald
     

  • Heith Seewald - 2015-12-17

    This looks good though Pranav -- I merged from pr/8013.

    Nice work :)

     

  • Dave Brondsema

    Dave Brondsema - 2016-04-11
    • Milestone: unreleased --> v1.4.0
     

Log in to post a comment.