amitaibu / og

A fork to work on OG8
https://github.com/Gizra/og
29 stars 16 forks source link

Subscribe and unsubscribe routes #268

Closed amitaibu closed 8 years ago

amitaibu commented 8 years ago

Restarting the great work from #59 , in a shared branch.

PR allows users to subscribe and unsubscribe from groups (group/node/1/subscribe and group/node/1/unsubscribe)

amitaibu commented 8 years ago

Update: I'm going to follow the pattern of node/add/{node_type}, as in fact subscribe is the OgMembership form with a few hidden fields.

amitaibu commented 8 years ago

Still very much WIP, but we have a subscribe page.

join_group___site-install

Right now I have changed it to GroupSubscribeForm extends ContentEntityForm because when we extend EntityConfirmFormBase the save errors with InvalidArgumentException: Field submit is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() which I still didn't find how to fix.

amitaibu commented 8 years ago

For now I'll override copyFormValuesToEntity so the form will submit. We'll have to figure out a better solution once we attach configurable fields (such as "request message" field for pending memberships).

amitaibu commented 8 years ago

Update: I've added the "request membership" field, and it now appears for needs approval memberships.

are_you_sure_you_want_to_join_the_group_abluo_enim_euismod_persto_velit____site-install

amitaibu commented 8 years ago

Unsubscribe is now also in. Next would be the tests.

This can already benefit from an early review.

amitaibu commented 8 years ago

Test is blocked by #278

amitaibu commented 8 years ago

This PR (along with its tests) helps uncover many issues which is great! #279 is the latest one.

amitaibu commented 8 years ago

Functionality, and subscribe form test are in. what remains:

amitaibu commented 8 years ago

I still suck at it, so hopefully it won't take too much time :)

Ok, that went easier than expected. So we're just left with the functional test. What could possibly go wrong? 😉

amitaibu commented 8 years ago

When I try to run functional tests on my local using ./vendor/phpunit/phpunit/phpunit --testsuite unit -c ./core/phpunit.xml.dist ./modules/og/tests/src/Functional/GroupSubscribeTest.php from Drupal root, I get the error:

1) Drupal\Tests\og\Functional\GroupSubscribeTest::testSubscribeAccess
Exception: Warning: mkdir(): Permission denied
Drupal\Component\PhpStorage\FileStorage->createDirectory()() (Line: 165)

However I can't seem to print out the directory it's unable to create. (and I've of course chmod 777 the entire sites/simpletest). Anyone familiar with this problem?

amitaibu commented 8 years ago

@pfrenssen did you happen to come across the above problem?

amitaibu commented 8 years ago

hmm, to answer myself — https://www.drupal.org/node/2760905

amitaibu commented 8 years ago

Finally the functional test for subscribe works :)

Next is the unsubscribe functional test..

amitaibu commented 8 years ago

So, yet another issue found by this PR - we didn't add the member role whenever we did $memebership->getRoles() - so permissions might not have worked. I'll open a follow up to add more test coverage.

amitaibu commented 8 years ago

The functional tests are in place.

I have moved the permissions from Og UI into OG core, since also the subscribe/ unsubscribe have moved to core itself -as they are such an import part of OG itself. @pfrenssen hope it's ok for you, otherwise let me know.

amitaibu commented 8 years ago

Ready for review! 😄

amitaibu commented 8 years ago

cc @pfrenssen

pfrenssen commented 8 years ago

Awesome work! I don't have time to review this today or this weekend unfortunately. I can take a look at this on Tuesday when I am at Drupalaton.

amitaibu commented 8 years ago

In order not to stop development, I wanted ahead an got it in without a proper review. "Post mortem" review would still be appreciated :)