Skip to content

Conversation

@taylor-work
Copy link

@taylor-work taylor-work commented Oct 23, 2020

What does it do?

This allows for custom session providers to allow configuration of their database connection. Currently if you configure the database you will receive an error. The ! was lost in this commit ed61633#diff-1998c9159f58f279c68ead712f31e89ec0056b400cb139c3b8f6afd71da9355d Looking at the commit comments that change seems unintentional.

Describe the technical changes you did.

Why is it needed?

This allows custom database settings to be set for the session middleware.

Describe the issue you are solving.
If you set

      session: {
        enabled: true,
        client: "postgresql",
        connection: "pg",
      }

will create this error '(middleware:session) please provide a valid connection for the session store' after this fix you will not receive this error and you will use the custom database connection for the session middleware.

Related issue(s)/PR(s)

Let us know if this is related to any issue/pull request

@taylor-work taylor-work requested a review from a team October 23, 2020 15:55
@codecov
Copy link

codecov bot commented Oct 23, 2020

Codecov Report

Merging #8457 (3a50d9f) into master (afaf111) will decrease coverage by 8.42%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8457      +/-   ##
==========================================
- Coverage   35.07%   26.65%   -8.43%     
==========================================
  Files        1308     1175     -133     
  Lines       14465    10149    -4316     
  Branches     1439      563     -876     
==========================================
- Hits         5074     2705    -2369     
+ Misses       8481     6950    -1531     
+ Partials      910      494     -416     
Flag Coverage Δ
front 26.65% <ø> (ø)
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...anager/admin/src/components/SelectWrapper/index.js 0.00% <ø> (ø)
packages/strapi-admin/domain/user.js
...ntent-type-builder/controllers/validation/types.js
packages/strapi-generate/lib/helpers/copy/index.js
...tent-manager/services/utils/configuration/index.js
...n/services/permission/permissions-manager/index.js
packages/strapi/lib/services/metrics/is-truthy.js
...t-type-builder/controllers/validation/component.js
...trapi-admin/services/permission/action-provider.js
...kages/strapi-admin/validation/common-validators.js
... and 124 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a68e7a1...4fdd533. Read the comment docs.

Signed-off-by: Taylor Zajicek <tzajicek@flexion.us>
@taylor-work taylor-work force-pushed the fix_custom_session_providers branch from f0d04c6 to 02c8068 Compare October 23, 2020 16:09
Signed-off-by: Taylor Zajicek <tzajicek@flexion.us>
@taylor-work taylor-work force-pushed the fix_custom_session_providers branch from f363fed to 8ac31ba Compare October 23, 2020 19:34
@taylor-work taylor-work changed the title Error if there is no database configuration for a custom session. Bug: Erroneous error if there is a database configuration for a custom session middleware storage provider. Oct 29, 2020
@strapi-cla
Copy link

strapi-cla commented Nov 30, 2020

CLA assistant check
All committers have signed the CLA.

@alexandrebodin alexandrebodin added source: core:strapi Source is core/strapi package issue: bug Issue reporting a bug labels Feb 3, 2021
@alexandrebodin alexandrebodin added this to the 3.4.6 milestone Feb 3, 2021
@alexandrebodin
Copy link
Member

Hi @taylor-work your PR has some linting issues :) can you fix those ?

Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@alexandrebodin alexandrebodin merged commit dfcab11 into strapi:master Feb 3, 2021
@taylor-work taylor-work deleted the fix_custom_session_providers branch February 3, 2021 16:55
@derrickmehaffy
Copy link
Member

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/new-release-strapi-v3-4-6/2631/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

issue: bug Issue reporting a bug source: core:strapi Source is core/strapi package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants