Forum OpenACS Development: Changes to OpenACS 5.10 from ]project-open[?
Here are some important fixes from ]po[ that we introduced since OpenACS 5.9.0. I removed anything ]po[ specific.
It's just 262 lines.
Cheers
Frank
The "report" is just 262 lines, because it is mostly undocumented and is using a stone-age diff
format (why are not using the unified diff, which makes it much easier to read and provides
a context). Applying undocumented and unclear changes is not a good idea, especially
when it might break other people's code or installations.
I took the effort and worked through the list of 14 changes and added some comments
to it. For some of these changes, i could find a possible explanation, for some not.
Please provide more details where necessary.
PS: why are you not using the OpenACS bug-tracker?
This allows detailed discussion and tracking for the issues on a per-package basis. Actually, it is not a good practice to bypass change request guidelines.
=======================================================
1) server-restarts
Restart doesn't work on Windows. Talk to Maurizio.
=======================================================
Based on a request from PO, the parameter "NsShutdownWithNonZeroExitCode"
was introduced in OpenACS 5.9.1 to handles the windows case
(PO should set this parameter). You have confirmed that
the command "ns_shutdown -restart" works.
It is not clear, why now the command should be run in
a scheduled thread, as the patch suggests.
[1] https://openacs.org/forums/message-view?message_id=5367113
=======================================================
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?
=======================================================
3) acs-bootstrap-installer
Tip from Brian. Cuts server startup time to half.
=======================================================
Generally deactivating automated testing does not make
sense and will break existing instances. However, in general
this is a sensible feature request to avoid loading the test files for
"production" sites .... i will look into this suggestion.
=======================================================
4) Content-Repository
Upgrade failed because of "dangling" entries.
=======================================================
This is a change in the update script from 5.9.0d1-5.9.0d2!
When this change would be applied, you will delete some
user content. This should not go into a general release,
since it can damage data in installations.
=======================================================
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.
=======================================================
6) acs-lang
Bad HTML...
=======================================================
What is the problem? The HTML code is according to HTML
validators correct (as checked by firefox). ... Are you saying,
we should insert deprecated tags?
=======================================================
7) mail-lite
$fixed_sender not respected.
=======================================================
Not sure, what the problem is, that you are trying to fix here.
When the package parameters "FixedSenderEmail" and
"OriginatorEmail" are set, then the fields are supplied with the
correct $fixed_sender value. If there is a problem, please explain.
# Decide which sender to use
set fixed_sender [parameter::get \
-parameter "FixedSenderEmail" \
-package_id $mail_package_id]
if { $fixed_sender ne "" && !$use_sender_p} {
set from_addr $fixed_sender
}
# Set originator header
set originator_email [parameter::get \
-parameter "OriginatorEmail" \
-package_id $mail_package_id]
...
switch -exact -- $originator_email {
fixed_sender {
if { $fixed_sender ne "" } {
set originator $fixed_sender
} elseif { $from_addr ne "" } {
set originator $from_addr
} else {
set originator $message_id
}
}
=======================================================
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?
=======================================================
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.
=======================================================
10) http-client-procs.tcl: Fix curl issue with insecure connections
@OpenACS: Please add accept_insecure_p or similar to util::http::curl::request.
=======================================================
The Use -k1 causes to accept blindly invalid certificates.
This should not be activated by default. However, this is
a sensible feature request to add an extra flag.
=======================================================
11) date-procs
Fix issue with ISO date.
=======================================================
What is the issue with the ISO date? How can the problem be
reproduced?
=======================================================
12) calendar/view-week-display
]po[ needs a base_url to point to a different package.
=======================================================
The diff is hard to read. can you provide a unified diff
(or context diff). The current code seems different and
seems to provide the base_url already.
=======================================================
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.
=======================================================
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.
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 dateWhat 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
Frank, could it be that these cases are when permission_p is referenced in a scheduled procedure?
I have run into this error a few times under these circumstances. In these cases, I find a way to pass the user_id with other parameters to the scheduled procs, or make a procedure dependency that doesn't reference permissions (and check permissions before scheduling). The scheduled proc doesn't have ad_conn properties.
cheers,
Ben
If it helps, once we upgrade to project-open 5.0 we would also upgrade to oacs-5-10 and then I can provide git based diffs using the bug tracker and proper channels.
Frank, just one comment though... Do you actually care that the code comes into OpenACS? If yes, then you might double check who is doing whom a favor without earning money and MBA Bullshitting.... just saying.....
