Restore default whitelist all get static strategy

[muuk-iO]

When you remove the default whitelists in the admin and restore them with the restore button, all the whitelist will be added with the static strategy.

Preconditions

Remove some or all whitelists with the regex strategy.

Magento Version : EE 2.4.5-p1

Force Login Module Version : 5.1.0

Steps to reproduce

1. install force login module
2. go to customers -> force customer login -> whitelist
3. remove

Expected result

1. all removed default whitelist are added.
2. all newly added whitelist will have the same strategy that were given when the module was first installed (in the patch file)

Actual result

1. all removed default whitelist are added.
2. all newly added whitelist have the static strategy.

[shochdoerfer]

Thanks for opening your first issue in this repo. Any chance you could help us fixing this?

[rikwillems]

I can confirm the behaviour as observed by @muuk-iO. The data patch uses a hardcoded whitelist while the restore feature uses an entry list injected through DI. This should be merged into one source of default entries.

[shochdoerfer]

@rikwillems makes sense. That would also mean the Patch classes need to filter/extract the values that need to be updated. Otherwise, db conflicts might be the result.

[rikwillems]

The patch is only executed once, during first install. Any database errors are already handled (ignored) through a try/catch in the patch. But, the patch does insert the right whitelist type. I'll try to align this behaviour with the "restore defaults" feature.

[shochdoerfer]

Exactly, that's the problem ;) If we want to add more default items to the whitepages list and a merchant has already installed a previous version of the module, then the newly added items won't get added to the database. Thus, we would need an additional patch file.

[rikwillems]

@shochdoerfer @muuk-iO What do you expect that a "restore defaults" button would do?

Currently it goes through existing entries and skips those, then adds non-existing entries to the whitelist table.

But, you could argue that a "restore defaults" button would truncate the whitelist table and add all defaults.

Also, the check for existing entries is only for url_rule and not for editable or strategy. In that sense, if you were to change one of these two values that rule wouldn't be restored to default.

Moving forward I would suggest to actually restore the defaults. So truncate the whitelist table and restore whitelist entries from the di.xml. I suggest to create a WhitelistDefaultProvider where whitelist entries can be registered but this and other modules.

Adding to the whitelist during a module update is debatable as it changes behaviour. It at lease would require separate patch files per version. We can't work from a default list here as merchants can also remove entries that would be re-added if we execute the default list during an upgrade. I believe we can think of a lot of interesting scenarios here.

[shochdoerfer]

@rikwillems I agree with your proposed way to restore the defaults. That makes the most sense, I guess. That would also imply that we have to work with multiple patch files in the future when we need/want to add new URLs to the whitelist.

[rikwillems]

@shochdoerfer I have a working setup ready and will create a concept PR for that.
We would need multiple patches indeed. Or don't add to the whitelist automatically after first install. Through the WhitelistDefaultProvider projects can add to the default list for first install. And do data patches on their own if necessary.

[shochdoerfer]

Just published v5.3.0 with a fix.