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

Fixed errors always empty #798

Closed

Conversation

circulon
Copy link
Contributor

Not really sure how this has stayed unnoticed for so long.

This fixes the "errors" put into session flash being removed at the beginning of the session and put into a view variable called "bag" for some reason.

The upshot of this is that the examples in the Validation documentation never work and the ShareErrorsInSessionMiddleware is always empty as it is loaded AFTER the session is established.

Not really sure how to add a test for this.

@eaguad1337 eaguad1337 self-assigned this Jul 5, 2024
circulon and others added 7 commits July 24, 2024 14:38
Refactored Redis Cache Driver
- Fixed internal cache not always loaded on first cace access
- added ability to define “timeout” for for item expiry
- - defaults to 1 month.  Previous hardcoded value was 10 years
- correctly store and unpack int types
- fix internal cache not removed if store was flushed
- fixed an issue where am imternal cache key would not be updated if it existed, but would be updated in the Redis store
Redis has no way to get key ttl enmasse to we oft for an on demand check and update the internal cache as required
@circulon
Copy link
Contributor Author

@josephmancuso @eaguad1337
has there been any evaluation of this fix?

This issue may be affecting many devs without them even realising it

@josephmancuso
Copy link
Member

i dont think we can do this because the documentation says to use the bag helper .. and if we modify this to be done right i think it'll be a breaking change

@josephmancuso josephmancuso changed the base branch from 4.0 to 5.x August 16, 2024 17:53
@circulon
Copy link
Contributor Author

circulon commented Aug 22, 2024

Hi @josephmancuso yeah you are right this would revert to the old behaviour without a MessageBag.
That said the current behaviour is still broken.

I have worked out the issue and am in the process of addressing it.
I will alsp add tests for the Session middleware.

here is what I have so far:

https://docs.masoniteproject.com/features/validation#displaying-errors-in-views
Shows to use the session().get('errors') method which always returns None
The errors variable in the View is only referenced as part of the SharedErrorsInSession

Additionally:
It would be nice to have the errors() helper to be backwards compatible and return a MessageBag instance

Current errors in view behaviour:

# file: index.html
...
session errors: {{ session().get('errors') }} {# always returns None #}                  
<br>                
session bag: {{ session().get('bag') }} {# always returns None #}
{# just proving there is no session var called 'bag' #}                  
<br>                  
view helper errors() : {# {{ errors().any() }} #} {# throws "errors() is undefined (when uncommented) #}                 
<br>                  
view helper bag() : {{ bag().errors() }} {# contains messages expected in errors() var #}
 ...

Expected behaviour:

# file: index.html
...
session errors: {{ session().get('errors') }} {# expected to return None, an empty dict or even a MessageBag instance #}                  
<br>                
session bag: {{ session().get('bag') }} {# always returns None #}
{# just proving there is no session var called 'bag' #}                  
<br>                  
view helper errors() : {{ errors().any() }}  {# Expected value: True #}                 
<br>                  
view helper bag() : {# {{ bag().errors() }}  #} {# Throws 'bag()' is undefined  (qhwn uncommented) #}
 ...

I hope this clarifies things

session flash now contains errors key as it did previously.
The errors are now a MessageBag which aligns the Session with the View helper ‘errors()’
@circulon circulon closed this Aug 22, 2024
@circulon circulon deleted the fix/errors_always_empty branch August 22, 2024 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants