pinax / pinax-messages

a Django app for allowing users of your site to send messages to each other
MIT License
202 stars 58 forks source link

NewMessageForm is broken in 1.11.7 #38

Closed volksman closed 6 years ago

volksman commented 6 years ago

The empty queryset breaks things.

to_user = UserModelChoiceField(queryset=get_user_model().objects.none)

should be

to_user = UserModelChoiceField(queryset=get_user_model().objects.none())

I have a PR (pr #37) setup to fix this AND remove dependancy on DUA

grahamu commented 6 years ago

Great catch @volksman!

FWIW, locally I added a test class:

class TestForms(BaseTest):

    def test_new_message_form(self):
        """Verify form instantiation without a hookset for `to_user` queryset"""
        NewMessageForm(user=self.brosner, initial={"to_user": self.brosner.pk})

    def test_new_message_form_multiple(self):
        """Verify form instantiation without a hookset for `to_user` queryset"""
        NewMessageFormMultiple(user=self.brosner, initial={"to_user": self.brosner.pk})

The tests don't really assert anything, but they do fail with an exception because the none() issue.

Since this fix has nothing to do with requiring login, I'd really like to see this fix in a separate PR. Separate PRs allows evaluation of each idea on its own merits.

I'll push my tests to master. Then, once you add a form only PR fix and everything passes I'll merge that PR to master with gratitude for your thoughtfulness. Thanks!

volksman commented 6 years ago

See PR #39

volksman commented 6 years ago

Above PR accepted and merged. Closing issue.