Opened 14 years ago
Last modified 6 years ago
#9612 closed enhancement
`WikiPage` doesn't need the `ipnr` attribute — at Version 29
Reported by: | Owned by: | Ryan J Ollos | |
---|---|---|---|
Priority: | low | Milestone: | 1.3.1 |
Component: | wiki system | Version: | 0.12-stable |
Severity: | minor | Keywords: | WikiPage model ipnr |
Cc: | Thijs Triemstra | Branch: | |
Release Notes: | |||
API Changes: |
Added |
||
Internal Changes: |
Description
Just saw this while changing th:WatchlistPlugin from own SQL code to the trac.wiki.model.WikiPage
class:
Instances of the WikiPage
class do not provide the ipnr
as attribute as for all other wiki page values (version, author, etc.). In fact the _fetch
method doesn't read this value from the DB.
If this is not by intention this should be fixed for consistency. Some plug-ins probably like to read this information out, e.g. I would have given the users the option to show it in their watchlist.
I would be willing to provide a patch.
Change History (29)
follow-up: 3 comment:1 by , 14 years ago
comment:2 by , 14 years ago
We removed the display of that information some time ago in 0.11-stable IIRC.
comment:3 by , 14 years ago
Replying to cboos:
Well, why do you need the ipnr? I always thought nobody was actually interested in that and thought that we should even remove it in later releases.
As I said, I'm writing a plugin which shows the wiki data to the user as a table and I just wanted to be as comprehensive as possible. If you think about this that way I will just drop it. It's not an important thing for the plugin.
follow-up: 5 comment:4 by , 14 years ago
Milestone: | → next-major-0.1X |
---|---|
Summary: | `WikiPage` instances lack the `ipnr` attribute → `WikiPage` doesn't need the `ipnr` attribute |
All clear then. Let's remember to remove the ipnr information from the wiki table, at some later point.
comment:5 by , 13 years ago
Cc: | added |
---|
Replying to cboos:
All clear then. Let's remember to remove the ipnr information from the wiki table, at some later point.
What version do you have in mind?
comment:6 by , 13 years ago
0.14 of course. I don't think we should do any more schema change at this point for 0.13, as we should slowly try to finalize this version.
comment:7 by , 13 years ago
Milestone: | next-major-0.1X → 0.14 |
---|
comment:8 by , 9 years ago
I was thinking we should add some notices to the documentation about the pending removal: log:rjollos.git:t9612-schedule-ipnr-removal.
comment:9 by , 9 years ago
comment:11 by , 9 years ago
Should we also remove
ipnr
from theAttachment
class?
Yeah. We should consistently remove attachment.ipnr
if wiki.ipnr
will be removed.
comment:12 by , 9 years ago
Milestone: | next-dev-1.1.x → next-major-releases |
---|
Thanks. I'll add notices to attachment
module about deprecation of ipnr
and pending removal, as in comment:8. Changing milestone since work will be done in milestone:1.3.1.
comment:13 by , 9 years ago
Related question, should we also remove [trac] show_ip_addresses option in 1.3.1?
comment:14 by , 9 years ago
Yeah, it looks like that should be removed. The only uses I see aside from adding the value to the Chrome
data dictionary are:
- tags/trac-1.0.3/trac/templates/diff_view.html@:65#L65
- tags/trac-1.0.3/trac/templates/history_view.html@:58#L58
Side note: I didn't realize it was possible to link to a specific option and highlight it. I guess that works with any wiki word or phrase though. Very cool!
comment:15 by , 9 years ago
Deprecated ipnr
attribute of attachment class in [13729:13730]. Deprecated show_ip_addresses
in [13731:13732].
comment:16 by , 9 years ago
Milestone: | next-major-releases → 1.3.1 |
---|
comment:17 by , 8 years ago
What about a hacked trac and where people upload unwanted attachments or alter wiki pages in ways we cannot imagine? Wouldn't it be nice to at least have the ip address from where the alteration or malicious upload came from?
comment:18 by , 8 years ago
For a publicly-accessible site, you probably want to be running SpamFilter. See also #12398.
For the specific case in which a site was hacked and you want to know the IP address, there are probably better ways to obtain that information such as logging from the web server.
comment:19 by , 8 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
follow-up: 22 comment:21 by , 8 years ago
LGTM, + 2 remarks:
- we should also remove
show_ip_addresses
in chrome.py - db42.py makes me think we could start to introduce a DDL helper functions, like
upgrade_table(old_Table, new_Table)
and a Table.remove_columns that would create a new Table instance, so that one could writeupgrade_table(attachment_table.remove_columns('ipnr'), attachment_table)
comment:22 by , 8 years ago
Replying to Christian Boos:
- we should also remove
show_ip_addresses
in chrome.py
show_ip_addresses
was removed in r14909. That change should probably have been committed with these changes.
- db42.py makes me think we could start to introduce a DDL helper functions, like
upgrade_table(old_Table, new_Table)
and a Table.remove_columns that would create a new Table instance, so that one could writeupgrade_table(attachment_table.remove_columns('ipnr'), attachment_table)
Similar aim in #11872. I'll look into it.
comment:23 by , 8 years ago
Proposed changes in log:rjollos.git:t9612.2. Some of the changes will be committed to 1.2-stable to address #11872.
follow-up: 25 comment:24 by , 8 years ago
I think the upgrade_tables
method should call update_sequence
if the table has an auto increment primary key.
follow-up: 26 comment:25 by , 8 years ago
Replying to Jun Omae:
I think the
upgrade_tables
method should callupdate_sequence
if the table has an auto increment primary key.
Is this only needed if the column with auto_increment
has changed, or has been removed? If the column with auto_increment
has not changed, then that column will have the same data in the new table as in the old.
comment:26 by , 8 years ago
Replying to Ryan J Ollos:
Replying to Jun Omae:
I think the
upgrade_tables
method should callupdate_sequence
if the table has an auto increment primary key.Is this only needed if the column with
auto_increment
has changed, or has been removed? If the column withauto_increment
has not changed, then that column will have the same data in the new table as in the old.
The upgrade_tables
re-create tables. Auto-increment column uses sequence object in PostgreSQL. Then, the sequence object would be re-created and started with 1. Inserting old records wouldn't change the sequence.
See also #8575.
comment:27 by , 8 years ago
[7ea7e2d5/rjollos.git] calls update_sequence
. I'll propose additional changes after #11872 is implemented.
comment:28 by , 8 years ago
API Changes: | modified (diff) |
---|
comment:29 by , 8 years ago
API Changes: | modified (diff) |
---|
Well, why do you need the ipnr? I always thought nobody was actually interested in that and thought that we should even remove it in later releases.