Make it possible to disable tenants #5050
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
javierm
force-pushed
the
hide_tenants
branch
3 times, most recently
from
04e5804
to
6afe487
Compare
javierm
force-pushed
the
hide_tenants
branch
from
6afe487
to
60ffc56
Compare
|
Hi again @javierm, While testing locally, I found disabling subdomains tenants works fine but disabling domain tenants is not working. I tested it in the development environment by adding the domain I gave to the tenant in my machine After digging, I found the cause could be in the |
javierm
force-pushed
the
hide_tenants
branch
from
60ffc56
to
67328f2
Compare
|
@Senen Great! I've updated the pull request so we raise an exception when accessing a hidden tenant with a domain, just like we do when accessing a hidden tenant with a subdomain. |
javierm
force-pushed
the
hide_tenants
branch
from
67328f2
to
b334f20
Compare
Senen
approved these changes
Dec 27, 2022
This is consistent with the way we use separate actions to hide and restore records, which is similar to enabling and disabling a record. We might do something similar with the `toggle_selection` actions in the future. For now, we're only doing it with budget phases because we're going to add a similar switch control to hide and restore tenants. We're also making these actions idempotent, so sending many requests to the same action will get the same result, which wasn't the case with the `toggle` action. Although it's a low probability case, the `toggle` action could result in disabling a phase when trying to enable it if someone else has enabled it between the time the page loaded and the time the admin clicked on the "enable" button.
We're going to use it in other places.
We were returning `nil` in this case, meaning we would be accessing the main tenant in this situation instead of returning "Not Found".
Note we could use `acts_as_paranoid` with the `without_default_scope` option, but we aren't doing so because it isn't possible to consider deleted records in uniqueness validations with the paranoia gem [1]. I've added tests for these cases so we don't accidentally add `acts_as_paranoid` in the future. Also note we're extracting a `RowComponent` because, when enabling/disabling a tenant, we're also enabling/disabling the link pointing to its URL, and so we need to update the URL column after the AJAX call. [1] See issues 285 and 319 in https://github.com/rubysherpas/paranoia/
javierm
force-pushed
the
hide_tenants
branch
from
b334f20
to
25435b0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Background
We don't allow removing a tenant because it would remove all its data, which would be a dangerous operation. However, there might be cases where we don't want a tenant to be accessible, and there's currently no way to do so.
Objectives
Visual Changes
Before these changes
After these changes