Posted by agentrickard on January 22, 2010 at 6:20pm
| Project: | Domain Access |
| Version: | 6.x-2.x-dev |
| Component: | User interface |
| Category: | feature request |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Do we need stronger warnings on forms controlled by Domain Conf, so that users don't accidentally reset values for the primary site?
See the current message in status.png. The other options are below.
Discuss.
| Attachment | Size |
|---|---|
| status.png | 47 KB |
| warning.png | 46.83 KB |
| error.png | 47.21 KB |
Comments
#1
Agreed. I would go with the error because it will cause errors somewhere by resetting values for the primary site.
#2
I agree that making this stronger would be helpful and agree that the error is probably the best way to go (though the warning may be more accurate in a sense). I also wonder if it is worth modifying the language to be more descriptive of what may go wrong. A couple of possible options include:
1. "You are viewing one.example.com. This form may need to be entered from example.com. If you save this form it will modify the settings for example.com, not one.example.com"
2. "You are viewing one.example.com. If you save this form it will modify the settings for example.com, not one.example.com. To edit the settings for one.example.com use example.com/admin/build/domain"
3. "You are viewing one.example.com. If you save this form it will modify the settings for example.com, not one.example.com"
#3
A version of #2, with link, is probably best. The problem is knowing what link to use.
#4
If you mean which link will be most understandable by the user, then I'd suggest linking to the actual settings page at /admin/build/domain/conf/DOMAIN_ID. That way they get the same basic page, which contains the information they are looking to change.
#5
There are times when hook_domainwarning() is used outside the context of settings, at least theoretically.
#6
Ah, yes of course - e.g., when /admin/build/themes is accessed from non-primary domains. So then I guess it's probably best to just use /admin/build/domain if that covers all of the possible uses of hook_domainwarning(). If that's still too specific then I wouldn't know what to suggest.
#7
Well, an API change that lets the hook declare its path, but short of that, just a notice.
#8
I've been thinking how to implement this without breaking other modules but there doesn't seem to be a way to do so. Modules implementing hook_domainwarning() will just need to keep up with DA.
We could simply return forms as key => uri pairs in hook_domainwarning() implementations but we would still need to perform additional check on that url the same way it happens in domain_get_uri() atm.
Perhaps the solution here is to:
1. make an adjustment so that url can be passed to to domain_get_uri() as an argument. If no valid argument is passed fall back to request_uri()
2. in domain_warning_check instead of in_array() use array_key_exists() and pass $forms[$form_id] as an argument to domain_get_uri() to get the accurate url to link to.
Can't roll a patch atm but that's one way to solve this problem.
#9
And a patch, which introduces an API change. Screenshot attached.
#10
Revised patch which catches both versions of the API and changes the wording slightly.
Committed.
#11
Automatically closed -- issue fixed for 2 weeks with no activity.