Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve session init and timeout handling #915

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from

Conversation

neddllly
Copy link

@neddllly neddllly commented Sep 13, 2023

Description

Enter a brief description of the bug being fixed.

Changes Made

Describe the changes made to fix the bug

Closes Issue(s)

Related Issue(s)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Automated testing implementation or update
  • Dependencies updated to a newer version
  • Documentation update
  • Experimental feature that requires further discussion

Screenshots and screen captures

@GhaziTriki GhaziTriki changed the title Main Improve session init and timeout handling Sep 13, 2023
@GhaziTriki GhaziTriki added the status: under testing Issue or pull request is currently being tested by the QA team or undergoing testing processes. label Sep 13, 2023
Copy link
Member

@GhaziTriki GhaziTriki left a comment

Choose a reason for hiding this comment

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

There are some rooms for improvement to avoid the duplication of before route that can be implemented in the parent Base action.

@marwa-benhassine marwa-benhassine added the quality control: changes requested Approved (for when the QA testing team approves the changes) label Sep 14, 2023
@marwa-benhassine
Copy link
Contributor

marwa-benhassine commented Sep 14, 2023

An error appears when running the project (installer)

image

Desktop (please complete the following information):

OS: Windows

Browser: Chrome/Edge/Opera/Firefox

@marwa-benhassine
Copy link
Contributor

This error appears when trying to login with a user that does not exist or with a user after registration

image

@marwa-benhassine
Copy link
Contributor

we should be blocked install process if and administrator user is available in the database .

install

@marwa-benhassine
Copy link
Contributor

After changing the admin name from the roles page, this error appears

Error

Class "Actions\Roles\User" not found

Administrator

@marwa-benhassine
Copy link
Contributor

Still the same problem as described above #915 (comment)

@neddllly
Copy link
Author

Still the same problem as described above #915 (comment)

there isnt anymore this function-> collectCurrentUser

@GhaziTriki GhaziTriki added this to the v1.0.0-alpha-2 milestone Sep 14, 2023
@GhaziTriki GhaziTriki added the priority: blocker Indicates that the issue or pull request is blocking progress and requires immediate attention. label Sep 14, 2023
@GhaziTriki
Copy link
Member

I had a discussion with @neddllly and it seems like there more consistency issues that needs to be fixed : #916. Testing will happen again on this PR when the necessary code is refactored.

@GhaziTriki GhaziTriki removed the status: under testing Issue or pull request is currently being tested by the QA team or undergoing testing processes. label Sep 14, 2023
bbbeasy-backend/app/src/Core/Session.php Outdated Show resolved Hide resolved
bbbeasy-backend/app/src/Core/Session.php Outdated Show resolved Hide resolved
@@ -99,7 +100,18 @@ public function set($key, $value): void
$this->f3->set('SESSION.' . $key, $value);
$this->f3->sync('SESSION');
}
public function getSession($sessionId)
Copy link
Member

Choose a reason for hiding this comment

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

This is still problematic in production mode since session data is stored in redis in production environment.

# Conflicts:
#	bbbeasy-backend/app/config/access.ini
@GhaziTriki
Copy link
Member

@marwa-benhassine Merges from PR #918 should fix edit issues. We can confirm with additional testing.

@marwa-benhassine
Copy link
Contributor

Okay Sir @GhaziTriki

@CherifAmine CherifAmine mentioned this pull request Sep 19, 2023
8 tasks
@hanazarraa hanazarraa mentioned this pull request Sep 19, 2023
8 tasks
@GhaziTriki GhaziTriki mentioned this pull request Sep 20, 2023
8 tasks
@sonarcloud
Copy link

sonarcloud bot commented Sep 20, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 12 Code Smells

No Coverage information No Coverage information
21.2% 21.2% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: blocker Indicates that the issue or pull request is blocking progress and requires immediate attention. quality control: changes requested Approved (for when the QA testing team approves the changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants