Hi Gustaf,
stone-age diff format
I have to admit we're quite technologically isolated here in Barcelona, and mainly dedicate our time to MBA bullshitting and extruding money from customers, instead of doing the cool coding that you do... 😊
Thank you very much to looking into these issues!
1) server-restart
I didn't remember about the previous issue. I'll try again. As above, my memory probably suffers from the MBA bullshitting process, where you try to stay "shallow" as much as possible. Or maybe just age?
2) authentication-procs.tcl - Why not accept email instead of username? It's so easy.
if someone want to use email addresses for login,
the parameter "UseEmailForLoginP" should be used.
Why is this surgery necessary?
The issue is relevant when using Active Directory LDAP authentication. There username is required (because of LDAP technical details).
Why not allow both? Our customers are "stupid" and don't get why we have to use email in the first hand. Both values are unique. You might actually drop the parameter.
3) acs-bootstrap-installer
I've seen your separate post on this, thanks. We will uninstall/disable acs-automatic-testing now.
4) Content-Repository - Upgrade failed because of "dangling" entries.
The 5.9.0d1-5.9.0d2 upgrade was completely broken before we added these cleanups!
No problem not to include this in the official OpenACS release, though.
Maybe include our stuff commented out in case of trouble for users?
5) DS - Wrong URL
This change is hard to read. If one activates the developer support,
and calls "ds_link" in the ds/shell, all URLs are ok, at least in OpenACS 5.10.
This is a simple syntax error! There is no variable $ds_urlrequest-info. This error only appears under some special conditions that I don't remember (see above, probably MBA bullshitting issue again 😊 )
6) acs-lang - Bad HTML.
OK, no problem.
7) mail-lite - $fixed_sender not respected.
In my 5.9.0 code, there is no switch statement. Instead, the originator is hard-coded to be the bounce_address:
set originator [bounce_address -user_id $rcpt_id -package_id $package_id -message_id $message_id]
Maybe this issue was already fixed in your version.
8) calendar JS - Fixed JavaScript error in case of empty date field.
How can this error be reproduced? Who says that the
user wants the current date, when no date is specified?
To reproduce just call in the context of an empty date field... Then cal.sel{M|Y|D} are obviously NULL.
user wants the current date
What other date to you want to use if the date field is empty? There is no other context.
This issue deals with a hard JS error in case of missing data! Everything is better than a hard JS error!
9) acs-permissions - Empty party_id causes a hard error in permission_p.
No. The actual code does not cause a hard error, but
uses the current user_id in such cases.
Rrright, current_user_id will be used. But it can also be empty in strange cases when authentication has expired or similar.
We had these cases several times, believe me, I've fixed this relatively recently. I'm not 100% how this happenes, but a 0 instead of "" seems reasonable to avoid hard errors.
10) http-client-procs.tcl: Fix curl issue with insecure connections
@OpenACS: Please add accept_insecure_p or similar to util::http::curl::request.
However, this is a sensible feature request to add an extra flag.
Thanks!
11) date-procs - Fix issue with ISO date.
What is the issue with the ISO date? How can the problem be reproduced?
This is very, very old. We add this piece of code there since OpenACS 3.4, I believe. I don't remember completely (see MBA bullshitting issues above 😊 ), but I believe it has to do if PostgreSQL returns a timestamptz instead of date, or the other way around.
I believe I asked to include this in OpenACS already some 10 years ago.
Without this patch, you can't create a new project in ]po[...
12) calendar/view-week-display - ]po[ needs a base_url to point to a different package.
Not that important, we have disabled the calendar display be default now.
13) file-storage - Installation breaks. Ignoring is OK.
Blindly ignoring errors is never the best solution.
Please provide a unified diff and explain the reason
behind the desired change.
Here is the proc. fs::delete_folder breaks probably because of database dependencies, similar to the content-repository upgrade script above. Maybe both may have the same root issue.
Fact is that the uninstall completely fails, which is not a desired behavior, I understand...
ad_proc -private fs::install::before_uninstantiate { {-package_id:required}} {} {
catch {
fs::delete_folder -folder_id [fs::get_root_folder -package_id $package_id] -no_notifications
} err_msg
}
14) file-storage - Unregister can throw a hard error in case of non-existing data. This breaks installation.
Blindly ignoring errors is never the best solution.
Please provide a unified diff and explain the reason
behind the desired change
This one has probably the same root issue as 13).
ad_proc -private fs::before_unmount { -package_id -node_id} {
Create root folder for package instance via Tcl callback.
} {
catch {
set folder_id [fs::get_root_folder -package_id $package_id]
oacs_dav::unregister_folder $folder_id $node_id
} err_msg
}
Again, thank you very much for looking into this.
Cheers
Frank
