Add a session example

Should this be a simple example for just setting and getting the session or more ? If more, I think we should have a outline of what this module should do.

Sounds good. Be sure it's internally documented and that it has tests. See also: #2209627: [meta] Module Checklist for Examples

First attempt without tests. Don't know what should be there in tests.

Adding for clearing the session.

Add tests and add some menu and task links.

Fixed some white spaces, and tried to fix toolbar test failure. Second test fail, I think is not relevant here.

Drupal Core changed. this is a different issue because original Examples module breaks the tests, not this patch.

Waiting for the following issues:
https://www.drupal.org/node/2547627
https://www.drupal.org/node/2173527#comment-10196477

We can more forward with the scope of this issue, which is making a session example.

Please open a new issue about the toolbar test.

I made it. The links are in tge #15

Aha.. Well you can add related issues as well. :-)

So what next ?

So what next ?

We are waiting for the answer. https://www.drupal.org/node/2173527#comment-10196477 We must to know it is a core bug, or we must update Drupal\examples\Tests\ExamplesTest test. If this test will be pass, we apply add_language.patch, and then this patch will be good again.

(I attache a patch which contains correction for all issues, do not use)

We are waiting for two related issues: #2547627, #2548027

As I mentioned, the issue with the non-passing test about the toolbar is unrelated to this issue. It's yet another branch blocker which we'll deal with. This is how it goes when you're trying to hit a moving target like Drupal 8. Ideally all tests would pass always, but that's not a goal we can shoot for.

So: Let's stay in scope and do reviews of the session example module and not poke around on the toolbar test in this issue. The best way to get some clarity on the aria-text issue is to address it in core, as with #2548027: Follow-up add back aria-label to toolbar tray

Here's an Examples issue for the toolbar test: #2505711: Fix toolbar tests, which will either require a patch or be closed when #2548027: Follow-up add back aria-label to toolbar tray is committed. (This is what I meant in #16 when I said to open another issue.)

Note: If you add square brackets around issue numbers, they magically turn into links to the issue in question: [#number]

Also, it's a very good idea to generate an interdiff, so reviewers can see what you've changed: https://www.drupal.org/documentation/git/interdiff

Setting this issue to Needs Work. Reviews of the module as presented in #12 forthcoming.

Thanks for all the work.

Some things this module needs:

A test to make sure the menu item appears on the page and leads to the form with a 200.

A test to make sure the session data is stored. For this, the test will need to create and log in a user.

Add a #placeholder to the textareas to give instructions rather than providing a default value. Something like 'enter your name here'

Some descriptive text on the session form. This should be an 'item' form element telling what will happen to the data being submitted.

Some descriptive text on the view page. Same deal: Tell the user what they're seeing and where it came from.

The name/email combination is pretty good, but let's throw in something like favorite color, just so it stands out that this can be any sort of information.

Needs a module file defining a documentation group, explaining what the module does. We need this so the module will show useful info on api.drupal.org.

And generally: See the checklist:#2209627: [meta] Module Checklist for Examples :-)

Also, I'm not sure whether it's necessary but we might need to remove the session information in hook_uninstall(). This means adding an .install file as well.

OK, so now that the branch blocker is fixed (more or less... #2505711: Fix toolbar tests) here's #12 without the changes to ExampleTest.

It still needs all the work from #26 through #28.

In test, we have 'when session is not present', 'session default value' on view page. But I think, there should also be a test when we submit value in post array. Currently we are submitting empty array in drupalPostForm().

Getting the ball rolling on this one again.

Did most of mile23's list except for adding the module file. Tests all run; having the user log in during the tests is unnecessary; the problem with the test was that the tab bar is a block, and needed to be explicitly added.

Change status so folks look at it, and the testbot too.

OK, looks like HEAD changed a little too much for the good of my patch...

Rerolling. Not sure if I'm doing the interdiff right under the circumstances, but I'm trying to make it against the state of #31.

Thanks for keeping it alive. :-)

Here's a review. I'll also start up a test to see what broke in the last little while.

1. +++ b/session_example/session_example.routing.yml
   @@ -0,0 +1,18 @@
   +    _access: 'TRUE'
   ...
   +    _access: 'TRUE'
   

   Maybe these need to be 'access content'

2. +++ b/session_example/src/Controller/SessionExampleController.php
   @@ -0,0 +1,44 @@
   +    $email = $session->get('session_example.email', $this->t('No EMail'));
   

   'No Email.'

3. +++ b/session_example/src/Controller/SessionExampleController.php
   @@ -0,0 +1,44 @@
   +      '#cache' => [
   +      'max-age' => 0,
   +      ]
   

   Ideally we'd have a cache context for our specific content within the session data, but I don't think we have that capability without creating it. That's too much for this example. But the cache should behave differently for different session IDs, so let's add the session cache context.

4. +++ b/session_example/src/Form/SessionExampleForm.php
   @@ -0,0 +1,161 @@
   +  /**
   +   * The Link generator.
   +   *
   +   * @var \Drupal\Core\Utility\LinkGeneratorInterface
   +   */
   +  protected $link_generator;
   ...
   +  public function __construct(RequestStack $request, LinkGeneratorInterface $link_generator) {
   

   FormBase already has LinkGeneratorTrait, so we should remove the $link_generator property and call setLinkGenerator() in the constructor.

5. +++ b/session_example/src/Form/SessionExampleForm.php
   @@ -0,0 +1,161 @@
   +  /**
   +   * Get the session object from here.
   +   *
   +   * @return \Symfony\Component\HttpFoundation\Session\SessionInterface
   +   */
   +  public function getExampleSession() {
   +    $request = $this->request->getCurrentRequest();
   +    $session = $request->getSession();
   +    return $session;
   +  }
   

   This needs a ton more inline code, since it's the thing we're demonstrating. :-)

6. +++ b/session_example/src/Tests/SessionExampleTest.php
   @@ -0,0 +1,82 @@
   +class SessionExampleTest extends WebTestBase {
   

   We should have a test that only sets the session without the form or controller.

   We should also test a behavior for logging in a user, setting their session, and then logging out. The session data should go away.

Manual re-roll.

Converted test to BTB.

The routes are no longer admin paths.

Updated some docblocks.

Added .module file with documentation.

Pared down the form and controller classes.

'View' page is now a table.

Still needs #39.1, 3, 6.

I changed my mind and want to demonstrate caching the view page.

Also needs:

Docs leading to the core docs for sessions: https://api.drupal.org/api/drupal/core%21core.api.php/group/session/8.5.x

Docs talking about stream_wrapper_example since it adds a session: scheme.

Re: #39.3, it seems that the cache is invalidated without declaring any #cache metadata. I think it's because changing session values ends up invalidating the user or maybe the cookie context. I'd much rather be able to explicitly state how this works in the example, so we'll finish this up when more research has been done.

I had only checked the caching locally using Bartik, which the testing system doesn't use, and that explains the fail in #43.

This version manages caching as a cache tag. I hope it's not too complicated.

One of the problems with a contrib module adding session data is that they should then remove it when the module is uninstalled.

Currently, I'm not entirely sure how to do this. I tried a few things... So I'll open a follow-up about this after committing here.

This is also a problem for the file wrapper example, but the file wrapper example allows arbitrarily large data, limited only by the data column in {session}.

Also need to add a session example component to the dropdown.

Follow-up: #2985471: Examples fill up your {session} table