Images are deleted during link approval process

[Alexander Obuhovich]

I've stumbled upon a problem during link approval (from Admin Console).

There are 2 category permissions:

• LINK.OWNER.MODIFY - changes, user makes to it's link are immediately seen on Front-End
• LINK.OWNER.MODIFY.PENDING - changes, user makes to it's link are stored in another link record for admin to approve/decline

When user has LINK.OWNER.MODIFY permission it works, like a charm. However, there are a problems in creating a link, that contains all changes made by user.

All files are copied normally. Images are also copied on disk (e.g. image_one.jpg will be copied to image_one_1.jpg), but original image filename stays in link record.

This way approving a link will delete all it's images from disk keeping broken records in Images table.

Since images are copied at least, then I suppose it's working partially. Then we need to ensure, that copied image filenames are put back into Images table record associated with the link, that will hold all modifications.

Another small problem here - such links (that needed to be approved) have broken icon in catalog. They should have Pending icon I suppose.

--
Best Regards,

http://www.in-portal.com
http://www.alex-time.com

[Phil -- wbtc.fr --]

Nice catch !

I'd suggest here 2 things:

- link ID shouldn't change, because we modify a link, and not creating a new one. we could use a temp linkID, such as 942-1, but the main thing is link ID should stay the same, otherwise we could rely on this record if it change every time a user post a change

- image filename shouldn't change, if there's no real change. along with temp linkID, we should keep original record when approving modifications

Sounds logical?

2011/7/22 Alexander Obuhovich <aik....@gmail.com>

--
You received this message because you are subscribed to the Google Groups "In-Portal Bugs Team" group.
To post to this group, send email to in-port...@googlegroups.com.
To unsubscribe from this group, send email to in-portal-bug...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/in-portal-bugs?hl=en.

[Alexander Obuhovich]

Sounds logical, but it's hard to implement.

In-Portal creates 2nd link, that holds all modifications and it's images are independent from original link (that is still shown on front-end at the time).

We can try to rollback filename change after approval was made however, e.g. try to remove "_1" from filename.

Rollbacking ID is problematic, since it's can be used elsewhere.

[Phil -- wbtc.fr --]

instead of rollbacking, why not copying, on approval, all datas from temp link to original link, and then delete this temp link? about filename, there shouldn't be a new file created in temp link if it already exist, don't you think so?

2011/7/22 Alexander Obuhovich <aik....@gmail.com>

[Alexander Obuhovich]

Coping is interesting idea, however don't keep a track of what was copied into what for in original and modified links.

[Phil -- wbtc.fr --]

how do we keep track of that actually? The design would be the same, show old and new infos, but at the end stay the original link, and new one is deleted after copying.

We could get this coding opportunity to display a new toolbar button "show only modified fields/Show all".

2011/7/22 Alexander Obuhovich <aik....@gmail.com>

[Alexander Obuhovich]

When link copy (with modification) is created then all images IDs in it are different from ones in original link. We don't know what image should replace what other image.

[Phil -- wbtc.fr --]

of course this wouldn't work for images... hmm but let me think about that: when new link (to be approved) is created, it have in images a mix of old images and new ones, that's why images are deleted at the end :)

Then if it keep record of old images, why not showing it? Isn't easy to compare images ID for a given link? New images have new ID, while old ones keep the same.

2011/7/22 Alexander Obuhovich <aik....@gmail.com>

[Alexander Obuhovich]

Nope, new images have new IDs. Each database record table have it's own unique ID.

[Phil -- wbtc.fr --]

Can't we avoid submitting images again when there's no new images, nor changes? This is relying on uploading system, right? I try to imagine a clever, resource-saver system.

2011/7/22 Alexander Obuhovich <aik....@gmail.com>

[Alexander Obuhovich]

Resources are ok, since original images are deleted after approval. Why you are concerned about link to keep it's id. It's not blog, that has tons of changes.

Link stays unchanged for a long time.

[Phil -- wbtc.fr --]

for example in my case, we use links to offer a "news" space for link owner. Then everytime these news are changed, we'd have a new link ID. It's not so problematic, but finally links ID are totally disconnect from total links count. More globally, it's not "logical" to create a new item when we "modify" it.

Dmitry, are you around?

2011/7/22 Alexander Obuhovich <aik....@gmail.com>

[Alexander Obuhovich]

We need to store all changed data somewhere? Yes, we should so declining a link will revert all back. Adding twice more columns to each table to store new value is a bad idea.

We create 2nd record instead.

Currently I have no idea how to implement what you are suggesting and I see no big need in it too.

[Phil -- wbtc.fr --]

Well, as usual I suggest improvement along with bug correction, but this is not the main thing here.

The only problem about filename is that if you create a new filename each time a link is modified, then the filename could end in filename_1_1_1_1_1_1_1.jpg .

My idea is to perform all things like now, until approval. On approval, instead of deleting old link and activate new one, I propose to delete old one and give new link the old ID we've just deleted.

I propose that because link pending approval is fine, images are correct (old images with old filename, no broken link to images).

What do you think?

2011/7/22 Alexander Obuhovich <aik....@gmail.com>

[Phil -- wbtc.fr --]

of course, because this is an approval process, we shouldn't delete old link images. Image cleaner agent will arrange images things anyway.

Isn't simple?

2011/7/22 Phil -- wbtc.fr -- <ph...@wbtc.fr>

[Phil -- wbtc.fr --]

Dmitry, I'll be glad to read your opinion here, plus having the pleasure to hear from you here :) 

[Phil -- wbtc.fr --]

ping

2011/8/16 Phil -- wbtc.fr -- <ph...@wbtc.fr>

[Dmitry A.]

Hi guys,

Sorry for delay.

I have carefully read through discussion. In my opinion we should NOT add any extra logic here and simply fix what's not working correctly in the first place.

In particular, Alex's suggestions should work well:

1. "Then we need to ensure, that copied image filenames are put back into Images table record associated with the link, that will hold all modifications."

2. Fix Pending Icon.

DA

[Phil -- wbtc.fr --]

Hi,

thanks for replying here. Maybe fixing what's wrong is the best way, but does the #1 means that on every link changes, images will have "_1" at the end of their name, because they have been copied?

Nobody see a problem to have LinkId changed... but doing so is not only confusing for admin, it also break many things in the system, starting by paid listing:

(reproduced in 513)

- set "Directory" folder permissions to "link.owner.modify.pending"

- create a link

- create a paid listing and apply it to the link you've just created

- go on front, modify the link

- from this point, paid listing is lost, because it use LinkId :-(

I don't see any logic in renaming an image which isn't changing (if it'd do like this), and I don't see any logic in changing LinkId, while we "modify" it, and not creating a new one.

What's your opinion on this guys?

p

2011/8/20 Dmitry A. <dand...@gmail.com>

--

You received this message because you are subscribed to the Google Groups "In-Portal Bugs Team" group.

To view this discussion on the web visit https://groups.google.com/d/msg/in-portal-bugs/-/3jkixYJoA94J.

[Alexander Obuhovich]

New link being created, that holds all new modifications looses original link enhancement.

That's bad.

Maybe coping data back (not LinkId and ResourceId fields) from new link (with modifications) to old link (that is enhanced) could solve the problem. And then we can delete new link, since all data was copied from it.

[Phil -- wbtc.fr --]

just what I've proposed before :)

The new link is just a temporary link, used to store change data and compare them with original link.

By the way, it's a pity that we don't have any tool to show only changed fields in link approval system. It list original and modified link fields, side to side, and when you have 50 customs fields, it's just impossible to check them all correctly.

What about images, will they have their name changed too?

2011/8/21 Alexander Obuhovich <aik....@gmail.com>

[Alexander Obuhovich]

Images is a problem in both ways of fixing this. I don't have any solution right now. User maybe replaced some image in changed link and how do we correlate with images from original link. 

Deleting original link images before replacing them with new link image won't be possible too, since they can overlap.

[Phil -- wbtc.fr --]

when user edit a link, he can do the following actions on images, tell me if I'm wrong:

- no change: why should we perform any change in DB/image folder?

- an image is deleted: we delete the corresponding record in DB, and maybe the file too (but could be deleted later by image cleaning agent)

- an image is added, we add the image, as it works now

Where is the problem here? It seems to me very complicated that the system does something else than this, don't you think so? And all of this weird behavior because of another weird choice: creating a new link when we just modify it. Then I propose to get rid of all these at the same time

2011/8/21 Alexander Obuhovich <aik....@gmail.com>

[Alexander Obuhovich]

Image storing system is a complicated one. In-Portal has 1 record in a database for every link image being uploaded. That record contains that image "alt text" and it's status (enabled/disabled), primary mark, name.

You can upload new image instead of existing image (by image field name, not by image filename) or you can upload new image at the end of the list.

Also since 2nd link is created to hold all changes, then it's logical that all non-changed data is duplicated too. This way 2 links became completely self sufficient. Making 2 links refer to the same image files leads to large problems, described in first post of this discussion.

Everything connected to category items is hard to understand, since most of it is just a heritage from In-Link 2.

It would be a lot easier to create direct fields in Link table to hold PrimaryImage & AdditionalImages. Then no problems, and no such fancy "alt for every image" feature.

As I told before there are too many variables involved. And every user uses In-Link differently. One users don't need that fancy image-based properties and some do need them.

[Phil -- wbtc.fr --]

thanks for this detailed information.

As I told sooner in this post, my users modify their links very often, and we end in image filenames like image_1_1_1_1_1_1_1_1_1_1_1_1_1.jpg, just because he modified 14 times his link. then after approx. 120 modifications, the filename will be 255 chars long, what will happens?

Then the only problem here is to have 2 links with same images. But finally the new link isn't intended to be a link, but rather a "temporary item" to store all modifications.

On deletion of this temporary link after approval or deny, could it do "delete the whole link but not images it refers to". If new added images aren't accepted, then they will stay in image repo until next cleanup.

This temporary link could be named for example LinkId=12 and temporary LinkId=12-

Even if some things comes from in-link2, why keeping way of doing which causes problems? I don't propose to remove any feature, I myself like and use image properties feature :)

2011/8/21 Alexander Obuhovich <aik....@gmail.com>

[Alexander Obuhovich]

I've investigated and it seems, that current moderation system (for links) only shows previous values of link record and it's custom fields.

After thinking about that for a while a few times I've got this idea:

1. create PreModerationValues field in Link and LinkCustomData table
2. new fields will store link & it's custom fields values before change
3. when value "before change" already exists for a given field, then don't overwrite
4. display changed values right away on the site, but set link status to "Pending Editing" status (from "Active")
5. on such link (in "Pending Editing" status) editing page show original values based on new PreModerationValues field
6. when link is Approved, then erase PreModerationValues field content
7. when link is Denied, then replace current link values & custom field values with the ones stored in PreModerationValues field

This approach:

• keeps backward compatibility - link approval page will look the same
• no new links are created on field change, when LINK.OWNER.MODIFY.PENDING permission is used
• makes "Pending Changed" links be visible on site
• don't create new images on every link change, but can't restore original images either

Hope, that somebody except me understood this, since there more technical details, then it should be.

[Phil -- wbtc.fr --]

I'm happy that you took time to think about a solution.

here is another proposal, using the actual system:

1. update enhancements & relationships to changed LinkId on approval
2. fix image problem by placing copied image names into changed link record (now changed link record have original image names, but images are copied on disk)
3. remove "_1" for filenames of approved link images

2011/8/22 Alexander Obuhovich <aik....@gmail.com>

[Alexander Obuhovich]

That one looks less time consuming (implementation time), then all our previous proposals.

[Phil -- wbtc.fr --]

it keeps actual system and just correct minor bugs. I hope images will feel good after this :)

2011/8/22 Alexander Obuhovich <aik....@gmail.com>

[Phil -- wbtc.fr --]

Dmitry, your advice here?

2011/8/22 Phil -- wbtc.fr -- <ph...@wbtc.fr>

[Phil -- wbtc.fr --]

I'd like to suggest an even easier system:

On new link approval, after original link deletion:

1. revert new link Id to old link Id
2. remove "_1" for filenames of approved link images (only apply to original images, not new ones)

It seems the easiest way to solve all 3 problems, I'm waiting your comments.

2011/8/24 Phil -- wbtc.fr -- <ph...@wbtc.fr>

[Dmitry A.]

Hi guys,

Let me say that I really liked Alex's idea about storing the Original Data values in some fields. To be honest I see no reason of getting CustomFieldTable involved here and I would do this a full throttle feature so it can be reused in the future for ANY items/records in general. All this is good, but would be VERY time consuming right now so while I believe idea is good we need to have more simple solution to the current problem.

At this point, I vote for a Phil's solution as this one:

1. update enhancements & relationships to changed LinkId on approval

2. fix image problem by placing copied image names into changed link record (now changed link record have original image names, but images are copied on disk)

3. remove "_1" for filenames of approved link images

As to the newer solution suggested by Phil - there might be some complications with LinkId, Alex what do you think on this?

DA

[Phil -- wbtc.fr --]

Thanks to have a look here.

My latest proposal solves the current problems > solution:

• LinkId is change on update > revert to original Id
• no need to update enhancements & relationships (your step #1)
• an update shouldn't lead to create a new item (lot more logical behavior)
  
  
• Link images in DB are not renamed correctly > change filename in system/images to make DB info consistent
• no need to change actual image management system (your step #2)
• it works right actually: why creating a new image when the original is to be kept?
  
  
• Approved images (your step #3)
• images have _1 because they are copied, and not because they are approved
• new images (I bet these are the one you are calling "approved") don't have any "_1" added by inportal, and thus if an uploaded image have this _1 in his original filename, it'll break the image link recorded in DB

I hope I'm right and clear :)

2011/9/12 Dmitry A. <dand...@gmail.com>

DA

--

You received this message because you are subscribed to the Google Groups "In-Portal Bugs Team" group.

To view this discussion on the web visit https://groups.google.com/d/msg/in-portal-bugs/-/c1iB1YUKGlQJ.

[Alexander Obuhovich]

1. from implementation viewpoint link id changing:
a. in 2 tables (my idea)
b. in 7 tables (Phil idea) - since link record data is spread across 7 tables and we also need to change resource id as well.

2. creating 2 copies of 1 image during link copy (with changes) creation maybe looks weird from end user (as Phil) viewpoint, but it's absolutely normal from development viewpoint. It is done to completely separate data of 2 link versions (with/without changes). In case if something is done with 1 link, then other link is not affected. You can see how bad it could be in current image processing.

3. my fix then

On Sep 12, 2011 1:19 PM, "Phil -- wbtc.fr --" <ph...@wbtc.fr> wrote:
> Thanks to have a look here.
>
> My latest proposal solves the current problems > solution:
>

> - LinkId is change on update > revert to original Id
> - no need to update enhancements & relationships (your step #1)
> - an update shouldn't lead to create a new item (lot more logical
> behavior)
>
> - Link images in DB are not renamed correctly > change filename in

> system/images to make DB info consistent

> - no need to change actual image management system (your step #2)
> - it works right actually: why creating a new image when the original
> is to be kept?
>
> - Approved images (your step #3)
> - images have _1 because they are copied, and not because they are
> approved
> - new images (I bet these are the one you are calling "approved")

[Phil -- wbtc.fr --]

4. alright then :)

2011/9/13 Alexander Obuhovich <aik....@gmail.com>

[Dmitry A.]

Hi Alex,

Are these separate solutions or steps/plan we have to take to fix the issue.

Sorry, I've been on/off with this discussion so I wanted to make sure I fully understand what we are talking about now.

Thanks.

[Alexander Obuhovich]

There are final version of what needs to be done:

1. update enhancements & relationships to changed LinkId/ResourceId on approval
2. make sure, that copied images are listed in "link to be approved" database record (instead of original images)
3. remove "_1" from link image filenames (when possible), once link was approved

You/me were about to create new task before your last post.

--

You received this message because you are subscribed to the Google Groups "In-Portal Bugs Team" group.

To view this discussion on the web visit https://groups.google.com/d/msg/in-portal-bugs/-/UuFkog5zlrYJ.

To post to this group, send email to in-port...@googlegroups.com.
To unsubscribe from this group, send email to in-portal-bug...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/in-portal-bugs?hl=en.

--

[Dmitry A.]

Hi Alex,

Would you please create a task for this one and properly describe there what needs to be done (plan).

Thanks.

DA

[Alexander Obuhovich]

Here is the task: http://tracker.in-portal.org/view.php?id=1142

--

You received this message because you are subscribed to the Google Groups "In-Portal Bugs Team" group.

To view this discussion on the web visit https://groups.google.com/d/msg/in-portal-bugs/-/vR7HhBCxS4kJ.

To post to this group, send email to in-port...@googlegroups.com.
To unsubscribe from this group, send email to in-portal-bug...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/in-portal-bugs?hl=en.

[Alexander Obuhovich]

Patch for only 2nd item ready to be tested (wrong images got deleted on link approve).

[Dmitry A.]

Thanks - looking forward to the other parts of the task.

DA

[Alexander Obuhovich]

During implementation of http://tracker.in-portal.org/view.php?id=43 task (https://groups.google.com/d/topic/in-portal-bugs/B-MB2l-Qijw/discussion discussion) I've found out, that changes, required for flash uploader to work (see "link_approve_removes_wrong_images.patch" patch), in a task http://tracker.in-portal.org/view.php?id=1142, associated with this discussion were not commited at all.

This way we not only need to apply missing "link_approve_removes_wrong_images.patch" patch, but only change it to work with latest flash uploader changes, made in http://tracker.in-portal.org/view.php?id=43 task.

On Thu, Dec 15, 2011 at 7:38 PM, Dmitry A. <dand...@gmail.com> wrote:

Thanks - looking forward to the other parts of the task.

DA

--

You received this message because you are subscribed to the Google Groups "In-Portal Bugs Team" group.

To view this discussion on the web visit https://groups.google.com/d/msg/in-portal-bugs/-/pNUoE6n89hQJ.

To post to this group, send email to in-port...@googlegroups.com.
To unsubscribe from this group, send email to in-portal-bug...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/in-portal-bugs?hl=en.

[Dmitry A.]

So what is the next step in this discussion?

DA

[Alexander Obuhovich]

I hoped this is obvious from my previous post. Let me explain it in shorted form.

• In 5.2.0-B1 we marked http://tracker.in-portal.org/view.php?id=1142 task as completed, however not all patches from it were applied.
• If we try to apply that patch now it won't work because of changes, made in http://tracker.in-portal.org/view.php?id=43 task.

Solution is obvious:

• create task in 5.2.0 version of In-Portal related to #1142 task
• attach adapted (according to changed in #43 task) patch from #1142 task
• commit them

On Sun, Jul 22, 2012 at 9:24 AM, Dmitry A. <dand...@gmail.com> wrote:

So what is the next step in this discussion?

DA

--

You received this message because you are subscribed to the Google Groups "In-Portal Bugs Team" group.

To view this discussion on the web visit https://groups.google.com/d/msg/in-portal-bugs/-/z3-QdDG7gkgJ.

To post to this group, send email to in-port...@googlegroups.com.
To unsubscribe from this group, send email to in-portal-bug...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/in-portal-bugs?hl=en.

[Dmitry A.]

Thank you for explaining and please create new task describing the issue and solution

DA

[Alexander Obuhovich]

Here is the task: http://tracker.in-portal.org/view.php?id=1369

On Mon, Jul 23, 2012 at 7:24 AM, Dmitry A. <dand...@gmail.com> wrote:

Thank you for explaining and please create new task describing the issue and solution

DA

--

You received this message because you are subscribed to the Google Groups "In-Portal Bugs Team" group.

To view this discussion on the web visit https://groups.google.com/d/msg/in-portal-bugs/-/bF_oB8iqs7MJ.

To post to this group, send email to in-port...@googlegroups.com.
To unsubscribe from this group, send email to in-portal-bug...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/in-portal-bugs?hl=en.