The testing via gitlab ci was already added in the 📌 Use GitLab CI for testing Fixed , so closing this one as duplicate.
Could confirm the bug, as it was always getting the global redirect setting and not user/role based logout setting. I just applied the patch to the issue fork for tests to run and easier maintaining so marking this issue as RTBC.
Based on the comments that could not replicate the issue and no activity in like a year, closing this issue as outdated. Please re-open if the issue is still present.
There was no new activity in about 4 years, so closing it as outdated.
This issue needs to be updated once 📌 Replace README.txt with README.md Needs review is merged, so postponing it until then.
Removed the Q&A that was added (by me) for the option to disable destination, as some other configuration changes will be added in the future and I think the first Q&A covers it with the configuration export (as all the new configuration features will be added to the new release), but did remove the to 1.4 in the Q as not sure what the new version will be, but basically applies for upgrading from 1.3 or 1.4 version. Maybe wording needs to be different for the Q (How to upgrade from version 1.3 to 1.4 or later?)? Not sure, so leaving it to needs review.
Tested, and it detects when writing in the ckeditor5 and does not log me out, so marking this as RTBC, and once merged, Autologout will support ckeditro5, until then, it does not (to answer the OP question).
The message can be changed/set to whatever the admin of the page wishes, but if we want to make the default value/configuration more user friendly like suggested, we need to make sure that the existing configuration also gets that change, that's why a post update hook was added. Leaving status to needs review, but from my POV, can be merged. (This change will also be present for at least D9.5)
Can you please use issue forks instead of patch files. The automated testing is enabled via GitLab CI and issue forks are preferred. Setting it to needs work.
This needs to be re-visited once ✨ Use Drupal.dialog call instead of jQuery dialog RTBC is committed, as there were some changed added that should solve the issue already, so for now postponing it.
The test added is passing, so see no issue with this being merged.
Tests are passing and I agree with the change to get the URL of user login page via route as this then guarantees tests are working everywhere.
Changed as much of JQuery to plain JS as I could, also removed the duplicated dialogs appearing when clicking on "Yes" button. There could be an issue of logging out after 10s after first "Yes" in D10, which is covered in 🐛 Drupal 10 second dialog logout Needs review . Needs manual testing to confirm everything is working as expected, the automated test are passing.
The getDestination() function which was getting the destination from referrer was always returning an empty string, which means the if block was always skipped. It did solve the warning but also broke the functionality of redirecting user back to its own profile (if for example user/2 was logged in and then logged out and user/3 logged in after, the user/3 was redirected to the user/2 profile, because of the destination). Adjusted the condition to basically always redirect to its own user profile, so we do not need to explicitly check if user and user id match based on destination, getting ride of a chunk of code. Also added a test that checks that the destination is set and after user/2 is logged out and user/3 is logged in, the user/3 is redirected to the correct user profile. Ready for review.
Good catch for the JS configuration. Just removed the casting from int as it's not necessary and updated post update hook to use correct variable for lifetime. The code works, it sets the cookie base on the settings defined in the configuration, so marking this as RTBC.
Added a simple test that simulates having more then two responses from autologout ajax, by adding it in first place in array, so hard coding keys is not an option. The code in js file works as expected by checking the correct key and based on that sets the response. Leaving it as needs review so somebody can check the test added.
The testing is done via GitLab CI. To not cause confusion, hiding the patch file (which is applied in the issue fork).
Corrected the CS issue for missing doc comment for the new getDestination function, testing is now done via GitLab CI, so test are passing now. Changing status to needs review so it can be re-tested.
Rebased the branch so it includes the new GitLab CI testing. Test are passing now, so ready to merge.
I added the post update hook for the new settings, so existing users have values set and they only need to export the configuration (to have it set "default"). And also applied the changes from patch in comment
#12
✨
Autologout cookie is not secure
Needs review
The testing is now done via GitLab CI and test are passing, so setting the status to needs review and "hidding" the patch files, as the issue fork exists and all changes should be done there.
Added the limit option to the test, ready for review.
The issue mentioned in the description was already corrected with 🐛 getRemainingTime treats logout_regardless_of_activity as always active RTBC . The role_settings that this patch also corrects is wrong. Closing this as duplicate in favor of other issue.
The test are passing for D9 and D10 with the latest changes to the dev branch, so this can be merged and the Drupal CI testing needs to be disabled so to primarily use the gitlab CI. Marking it as RTBC, as there should be nothing to review.
I re-tested again to see the behaviour and on D9.5 works fine (except for multiple dialogs adding on when you interact and extend the session by click on yes). But that issue should be resolved with 🐛 Autologout triggered in background tab, no indication in other open tabs Fixed . For things to move forward, I think this needs to be merged and then new issues can be opened and solved, so marking it as RTBC.
Tests for D10 are failing, and also some for D9. The ones for migration are expected to fail because of a core issue 🌱 [META] Serialization issues in Migration tests Active , the others are not. Setting this to needs work, just so it's clear this is still being worked on. But the gitlab CI seems to work as expected, so maybe a new issue is needed? Not sure.
Added additional Q and A for ✨ Option to disable destination Needs review .
Setting status to postponed so it can be revisited once the related issue is merged, to test if the correction is still needed.
This is a specific case that basically should not happen, as the redirect url is currently a required field. But in the ✨ Option to disable destination Needs review an option to disable the destination url was added, so this issue needs to be re-checked once that is merged, to see if the fix is necessary. So postponing it for now.
Pushed the code to issue fork for easier maintenance. The code does work, the hook (when added) is triggered if user is automatically logged out. Leaving it to RTBC.
Rebased it again and added the update hook that sets the include destination to true. As existing users will not yet have that option and with updated with this change, it will default to false, which we don't want.
The patch was not applying, but instead of re-rolling added the code to issue fork for easier maintaining. The code works, so in my POV can be merged, but will leave it to needs review. Tests are expected to fail for migrations 🌱 [META] Serialization issues in Migration tests Active .
Added the patch to issue fork for easier maintaining. The tests are failing because of the core migration issue 🌱 [META] Serialization issues in Migration tests Active , but the code provided does seem to work, but setting the status to needs review, so somebody else can also again re-test and confirm it's working as expected.
The MR #27 can be closed as all the changes were happening in the MR #26. I added a FAQ section based on the 🐛 Autologout disabled after 1.4 Upgrade Closed: works as designed , ready for review.
I tested this and the update is running as expected and setting the enabled to true, the issue happens when you then do the drush cim, which gets the old configuration that does not have that field, so it sets it back to disabled or rather NULL, basically overrides the drush updb changes.
The solution to this is, do the drush updb, then do the drush cex so you can get the new configuration with the new enabled field.
Maybe this step needs to be added to the readme or the documentation page, to make it more clear.
But the updating of the field works as designed, so closing this issue as such.
Applied the patch from comment #8 🐛 Automated logout functionality not working, due to ajaxing property return true and ajaxStop event not triggering even though ajax req completed. Needs review and created a MR (so it easier to maintain). Tested the functionality which seems to work as expected. The tests are expected to fail because of a core migration issue 🌱 [META] Serialization issues in Migration tests Active , so see no reason this can be merged, but changing status to needs review, so somebody else can also test.
DeaOm → made their first commit to this issue’s fork.
It seems this issue is handled in 🐛 Autologout triggered in background tab, no indication in other open tabs Fixed , so closing as duplicated one, but feel free to re-open if that is not the case.
Tested the code form MR (which is the same as in patch) and it works. Changed the code to use request stack instead of the global php session and cookie.
If I have multiple tabs open and on the last active one I am logged out (either by the pop up window or without it) I am also logged out from other tabs. If I have the pop up enabled and I trigger the "keep me logged in" via clicking the yes button, I am also not logged out from other tabs.
@pankajsachdeva Can you provide more information on what is not working for you?
Also the code from here is basically the same as in the #3319047: Autologout does not consider activity while creating node and adding content on ckeditor of paragraph. And autlogout manager getRmaining time is having improper code to fetch logout_regardless_of_activity data → as mentioned in the comment #26 🐛 Autologout triggered in background tab, no indication in other open tabs Fixed . It seems this one is a little bit more active, but for the maintinaers to give credit also on that issue, it needs to stay open.
This issue also confirms the issue with the setting of the logout time to one minute as it's not working (you are always logged out after 2 minutes). But that is handled in 🐛 Undetected request prevents logout Needs review which will need to be updated once this is merged as there are some changes in the JS file.
This issue is really strange as it seem it is only happening with the 60 setting, even if you enable the pop up to show. If you set it to anything grater like 80 or 120 it works as expected. Tested different possibilities and the one worked was the changing of the timer for resting the activity from one minute to a half of minute. Tested manually and the user gets logged out after one minute when that is set, also user gets normally logged out for grater times, like 2 minutes. I also added a test that demonstrated that, but the timeout there is se to 15 seconds, so the test does not take too much time to complete. Locally I also tested with the 60 setting in test and it's also working as expected.
The test for migration should fail here because of the core issue with that
🌱
[META] Serialization issues in Migration tests
Active
.
It also seems these are duplicates 🐛 Throws Notice on Login when destination is set Needs work and 🐛 Autologout does not handle multiple destinations Needs work .
The tests that are failing seem to stem from a core issue 🌱 [META] Serialization issues in Migration tests Active . So either this issue can be blocked until the core issue is fixed, or based on the reviews that it's working can be merged. Leaving it up to maintainers and leaving the status as is.
It seems this is a duplicate of the 🐛 Warning: Undefined array key 1 in Drupal\autologout\EventSubscriber\AutologoutSubscriber->onRequest() Needs work or at least it seems it has a cleaner fix for it.
The 2.0 supports D8 and D9, and the version 3.0 supports D9.1 and D10. So the fixes are not needed for 2.0 as it will not support D10.
I was trying to reproduce this, but can't. I do have DB server and an index installed besides the Meilisearch one. The DB and Solr should not affect faceting of Meilisearch. We did however had some bugs regarding the filterable attributes.
Did you try clearing all the indexed data and re-index again?
Could you maybe try with the 2.x version, just so we can see if it's happening there too? The 2.x uses a more stable Meilisearch version 1.3.3 and there are some bug fixes applied, that could possible already solve this issue.
The getting the config part was working only in the instance your server name was meilisearch, if you named it anything else, the config was empty and therefore an error was thrown as there was no backend_config which means the master key was set to null. Added a helper backend trait that basically gets the backend configuration to then set the meilisearch api with the backed configuration host and key.
I did add the facets under filter criteria of the search view and tested the functionality and it seems it's working correctly. I added a fulltext type to a title in search api, the facets are also set up to a field title. If there is no term in the search box, all the facets are shown with the results in the brackets (basically all need to have a number higher then 0). Then once I enter a search term in the search box, I again get all the facets, but this time the brackets number changes to the "result hits".
Ready for review again.
Thank you for looking into it. Did test the code and it's working as expected and I see no issue with this being sorted before comparison is made, marking it as RTBC.
I do agree for the order not being important for the searchable and filterable attributes, but for sortable it's important that they keep the order in which they are set in the search API fields, as if I'm not mistaken, the first one added (on top) is the one with more "importance" when sorting, but I could be wrong, so leaving this to needs review.
Tests were failing on pipeline for D10, for some strange reason, not connected to the code, so I just triggered the pipeline again and now the tests are passing. They are also passing locally for me, which means the removal of code did not break anything and was obsolete. Marking it as RTBC.
The custom suggester did not make sense for what meili provides and what is already implemented. But because not all users will want to enable autocomplete, moving it to a sub-module was the only viable option, because of the dependency to search_api_autocomplete (now it's only needed if you want to enable the search_api_meilisearch_autocomplete module.)
Ready for review.
Locally test are passing for me, but for whatever reason they are not here, so setting the status to needs work.
Re-tested because tests were passing, so manual testing needed to be wrong and it was. Basically the fulltext type was set on the wrong field. After changing that, everything works as it should, so marking it RTBC.
The tests are also passing locally, but when manually testing, not sure if I'm doing something wrong, but if I add the "search full text search" with the empty field filter, I don't get correct results, if I search with the "full text search filter empty" then I get results if filter is also empty, when testing is empty.
For example, entity title is New entity, subtitle for this entity is empty. The condition are search full text New entity, subtitle filter is empty. I would expect to only get New entity as a result, but I get no results. If I then leave the full text search filter also empty and of course also subtitle, I then get all entities, that have empty subtitle. This part then works correctly. Not sure if it should work together like that. So leaving to needs review for now. If this is how it's suppose to work, then see no issues with this being merged.
Tested manually and the results are correct, tests are also passing, so see no issue for this to not be merged, RTBC.
Updated the readme file, ready for review.
Updated the version compatibility, get started link and maintainers and also removed the troubleshooting section. The versions section will need to be updated again, once the new version is released.
The new version of meili changed their naming from MeiliSearch to Meilisearch and the namespaces were not updated to match that, so changed that. Also updated one see link, to point to the new documentation. For whatever reason, locally the QueryFilterTest the testFiltersWithSpecialCharacters is failing for me, but here it seems it passes. I'm not to worried about it, as we do have an issue where this will be retested again. From my POV can be merged, but will leave it to needs review.
Removed the part of code, that removes the relationship from view, as that means that users can't place a block anymore even if they set the content types again, if they do not add a relationship back again or disable and then enable the module again. The block gets deleted/removed from being placed so it's no longer present on the page and therefore there is no error when revisiting pages that were added to the block basically. If users decide to again place the block, they can do so, by first setting entity type back in the configuration. If they do not, the same problem appears. Not sure if this should be handled here or not, to not allow placing a block if there were entities already added. Leaving it open for discussion.
The count comparison in removed, ready for review.
Added a test, that should fail, to expose the issue.
Added an additional check in the supportsIndex for server. Needs review.
The one issue I also encountered is, if you already have a meili server set up with an index that already has a field with the "id" name and you want to reindex it, it will reindex it, which is not correct. So added an event subscriber that checks if there is a field with the id set up, to prevent the indexing of that index.
Corrected order of use statements and also removed static calls for loading entities, added type hint etc. Needs review as there are not adequate test, to see if anything broke.
Applied the patch to issue fork for easier merge and opened MR.
Changing the status back to RTBC, so maintainers can give credit.
The MR seems to be merged, but the status was not changed to fixed, so just updating the status.
Updated maintainers to currently listed ones and added the Introduction section. Ready to be merged.
The branch is rebased so it's ready to be merged.
Changed the primary key back to id and added the validation also for index save. When you for example have no server selected or some other server that is not meili and the field is changed to id, then you decide to change the server to meili ti should throw an error, and not allow to save it. The validation for index field was added from the 🐛 Remove the possibility to add field with machine name id Fixed . Ready for testing.