Project

General

Profile

Actions

Bug #16166

closed

Publishing a record breaks dam_ttcontent-relations

Added by Andreas Wolf almost 18 years ago. Updated over 10 years ago.

Status:
Closed
Priority:
Must have
Category:
Workspaces
Target version:
-
Start date:
2006-05-22
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
4.1
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

When you publish an updated version of a record, changes in the DAM-Items-Field (tx_damttcontent_files) are not shown in the FE. You always get the images you inserted into the first version of the record. This happens because the field uid_foreign in tx_dam_mm_ref is not updated when the id of the two content-elements (the old "live"- and the new "draft"-element) is switched.

Create a new content-element in a draft-workspace, insert DAM-items (with extension dam_ttcontent) and publish it. You won't see the images because the version you edited in fact already was a second version, the first version was created in the live-workspace. Or, alternatively, edit an already existing item in a draft-workspace - you will experience the same behaviour.

I have attached patchfiles for the following extensions and classes:

- tx_dam (tx_dam_db)
- dam_ttcontent
- t3lib_tcemain

I created a new hook in t3lib_tcemain->version_swap to be called after two versions of an element have been swapped. A new function in tx_dam_db (swapMMrefs) is called by this hook. The function checks if the modified table is tt_content and if so, then switches the records in tx_dam_mm_ref.
(issue imported from #M3531)


Files

patchfiles-3531-fixed2.tar (10 KB) patchfiles-3531-fixed2.tar Administrator Admin, 2006-07-04 15:56
t3lib_tcemain_publish_and_swap.diff (2.15 KB) t3lib_tcemain_publish_and_swap.diff Administrator Admin, 2006-07-04 16:27
3531_typo3_4.1_dam_1.1_beta5.tar (10 KB) 3531_typo3_4.1_dam_1.1_beta5.tar Administrator Admin, 2007-03-19 00:20
3531_dam.diff (898 Bytes) 3531_dam.diff Administrator Admin, 2007-04-11 21:59
3531_dam_ttcontent.diff (1.15 KB) 3531_dam_ttcontent.diff Administrator Admin, 2007-04-11 21:59
3531_t3lib_tcemain.diff (1.63 KB) 3531_t3lib_tcemain.diff Administrator Admin, 2007-04-11 21:59
3531_t3lib_tcemain-without-hook.diff (1.02 KB) 3531_t3lib_tcemain-without-hook.diff Administrator Admin, 2007-04-13 20:40
3531_t3lib_tcemain-all-tables.diff (1.19 KB) 3531_t3lib_tcemain-all-tables.diff Administrator Admin, 2007-07-25 21:20
3531_t3lib_tcemain-all-tables2.diff (1.46 KB) 3531_t3lib_tcemain-all-tables2.diff Administrator Admin, 2007-07-25 22:03
3531_t3lib_tcemain-all-tables3.diff (1.53 KB) 3531_t3lib_tcemain-all-tables3.diff Administrator Admin, 2007-07-25 22:29
MMetusalem.rtf (3.52 KB) MMetusalem.rtf Administrator Admin, 2008-01-31 23:59
MMetusalem.diff (16.4 KB) MMetusalem.diff Administrator Admin, 2008-02-01 00:00
mmetusalem_4-1.patch (16 KB) mmetusalem_4-1.patch Administrator Admin, 2008-02-01 13:52

Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Bug #16393: Workspaces loses MM relations on saving a new record and publish a draft record.ClosedIngo Renner2006-07-21

Actions
Actions #1

Updated by Andreas Wolf almost 18 years ago

The patch I initially submitted contained an error. Please do only use the newer, fixed version!

Actions #2

Updated by Julien Bourdin almost 18 years ago

Seems to me that $TYPO3_CONF_VARS isn't set in the scope of the method version_swap in t3lib_tcemain so the patch doesn't work for me.

I added a "global $TYPO3_CONF_VARS; " before
"if (is_array($TYPO3_CONF_VARS..."

Seems okay now for swapping. For a new content element with dam image, publishing still break the relation.

Actions #3

Updated by Andreas Wolf almost 18 years ago

Julien, you are right - I didn't update class.t3lib_tcemain.php in my second patch-tar, though the error I wanted to correct with the fixed patch lay in this class. I now uplodaded a new version with an updated t3lib_tcemain.php

Actions #4

Updated by Julien Bourdin almost 18 years ago

To publish without swapping and have the dam relation linking the good tt_content, I propose to use the same mod for t3lib_tcemain::version_clearWSID()
I've uploaded "t3lib_tcemain_publish_and_swap.diff" in that aim.

Actions #5

Updated by Andreas Wolf over 17 years ago

Julien, I'm not 100% sure, but I think, you don't need to differentiate between publishing and swapping - it is enough if two versions are swapped, and this always happens, whether you publish a record or swap two records.

If you publish a version over another one, the old record/version is send to archive, if you swap two versions, the old live-version is made editable. I think - without having looked at your or my code - that both cases are covered by my patch already.
I didn't notice any problems with simple publishing and my patch until now, and it's in wide usage here.

btw. could one of the admins please delete my two old patches? Thanks!

Actions #6

Updated by René Fritz over 17 years ago

Could you please test with the latest mmforeign version from here http://typo3lab.colorcube.de/dam.html
and tell if there's still a patch needed. Thanks!

Actions #7

Updated by Julien Bourdin over 17 years ago

After a few test, it seems to me that nothing is changed to handle the workspace bug.
In the actual structure of database, only the tx_dam_mm_ref table has information on wich dam elements is linked to a tt_content. since publishing a versionnized tt_content implied a swap of uid, the same swap is needed in tx_dam_mm_ref.

Also, the preview of a workspace is broken and use the live related dam content.

The Andreas Patch correct these two problems and is still needed.

update:

BTW, Andreas Patch is okay, my file is not needed. Can it be deleted?

If your planning to use tx_dam_mm_ref for other table than tt_content, you will need to had a "tablenames='tt_content'" in the three requests of swapMMrefs.

Actions #8

Updated by Andreas Wolf over 17 years ago

Important! If this patch doesn't work for you, please check if you have applied patch #0003544 and if yes, remove it and re-apply the new version by Michael Stucki. My latest patch for 3544 contained a duplicate hook-call which originally belongs to this bug - and if both are applied, the version_swap-hook is called twice, so the ids are changed twice: orig -> new -> orig

Edit: btw, the new version of mmforeign doesn't change anything.

Actions #9

Updated by Andreas Wolf about 17 years ago

This problem is not fixed in the latest DAM-snapshots, and another problem has arisen: When adding DAM-elements to a draft-version of a CE, the references are stored with the id of this draft-element, but the DAM searches for the uid of the live-record.

I will further investigate this later on.

Actions #10

Updated by René Fritz about 17 years ago

It seems to me that this is a general (conceptual) problem of workspaces and not DAM specicific.

Anyway I would like to provide a solution for DAM, but I don't like to patch tcemain inside the dam extension.

Can anybody guide me what to do?

- Should I include the patch with the tx_dam_db->swapMMrefs() and a second extension make an XCLASS of tcemain and provide the hook?

- Is there a solution with T3 4.1 already? I haven't found one.

- The patch does work with tt_content only. What about other tables?

Actions #11

Updated by Andreas Wolf about 17 years ago

I did not think about other tables, neither did I test it with T3 4.1 and DAM Beta 1.1. At the moment I don't have my laptop (main development system), so I can't test it there - but I'll test it on another PC later today.

With other tables the behaviour should be the same - perhaps we would need to introduce a new configuration option or make it generally available for all tables. I didn't look at this issue for a rather long time so I'm not really into it at the moment... ;)

Actions #12

Updated by Andreas Wolf about 17 years ago

Well, here is what I found out (all based on DAM 1.1 Beta 5 as published by Rene):

a) Images inserted into CEs are not displayed in draft-workspaces. I suppose this is a general problem with mm-refs for DAM. I could fix it by re-applying the following patch to class.tx_dam_db.php (in l. 742):

// if we are previewing a CE in a draft-workspace, then use the uid of this
// CE and not the uid it will be published under
if ($this->cObj->data['_ORIG_uid'] && $this->cObj->data['_ORIG_uid'] != $this->cObj->data['uid']) {
$foreign_uid = $this->cObj->data['_ORIG_uid'];
}

b) The foreign_uid of the mm-relation is not changed when swapping the record into the live-workspace. This is also changed by the enhancements I used in my patch.

So, with a modified (i.e. updated version) of my patch this issue will be fixed. I will create and upload it later on.

@René Fritz: I think the changes in t3lib_tcemain will be neccessary, either in an xclass or in the core itself. I would opt for the core-version because an extension that is as important as DAM should not rely on X-classes too much IMHO.

Edit: I'm unsure about the other tables - I would normally say to totally disable the check I have in swapMMrefs at the moment. This would enable the swapping for all tables. But on the other hand this would be a bit risky - nobody knows when the function t3lib_tcemain::version_swap, which in turn calls swapMMrefs, is called. So this would have to be thoroughly tested...

Actions #13

Updated by René Fritz about 17 years ago

After discussion with other's I decided not to include such a TYPO3 core patch into the DAM extension. The code does not patch DAM but TYPO3!
Also the code can be placed into a second 'workaround'-extension.

Actions #14

Updated by Andreas Wolf about 17 years ago

Well, I disagree here - it should not be integrated into DAM, but into the core directly! DAM is one of the main components of TYPO3 for enterprise usage, so it has to work without such serious errors...

XCLASSing could be an option, but only for a limited time, not forever. XCLASSING is a more or less ugly hack for me, so it should only be used in narrow-usage scenarios, where integration of the changes into core doesn't make sense or would bloat the code too much. But both is not the case for this bug, so the patch should be integrated into core.

I will write an email to the bug-team-list about this later on.

Actions #15

Updated by René Fritz about 17 years ago

Well, I disagree here - it should not be integrated into DAM, but into the core directly!

That's what I said - more or less :-)

Actions #16

Updated by Andreas Wolf about 17 years ago

Ok, and I meant I disagree with "wil not be fixed"... :-)

btw: I wrote an email to the bugs-list and Masi pointed out that indeed this error should be completely fixed in the core as it is a general bug and not only related to DAM. I didn't think of this before, but it seems logical to me - and as a consequence this would mean to remove my changes from DAM again and move them to TCEmain.
I will test this later this evening and post my results here and to the bugs-list :) So be prepared to undo my rubbish again ;)

Actions #17

Updated by René Fritz about 17 years ago

Thanks for your effort!
I may not follow discussions about that core fix, so please give me a hint when there's a solution which can be described in the dam manual.

Actions #18

Updated by Andreas Wolf about 17 years ago

Ok, it has been a bit of work, but hey, TV was boring, so I needed a better occupation ;)

Everything works fine now (at least what I tested), the function swapMMrefs has been removed - we don't need it anymore, the functionality is in TCEmain now.
Additionally, I have fixed a bug in dam_ttcontent which resulted in the images of the live-version always being displayed in the page-view, instead of the ones of the draft-version if one exists (the same problem as with the FE-view). This is the reason why we still need a patch for dam_ttcontent.

I have attached three patchfiles now, as this is easier to use (Masi pointed this out). Rene, before you apply them, please remove my changes from the source completely. The patches should be applicable to the latest version of DAM (1.1 RC 1), although I only tested it with 1.1 beta 5.

I will post a message to the bugs-list in a few minutes.

Actions #19

Updated by Andreas Wolf about 17 years ago

I have removed the hook now as it is not neccessary with the new patches - see <> and <>.

Actions #20

Updated by Andreas Wolf almost 17 years ago

Well, after more than three months I have finally managed to get back to this bug again - and here I am with a new solution, this time globally for all tables.

The trick is that in version_swap the TCA is checked for MM-colums now. So the patch does not only apply to tt_content and the colum dam_ttcontent_files, but to all colums which are marked as MM-cols.
The only problem I possibly see is this: What happens with tables that are not versioned? But I think this should be no problem either because for these tables version_swap will never be called.

So, please test this patch as widely as possible, especially with other MM-enabled extensions. I will do so next week with a customers installations when we prepare a big upgrade (T3 + most extensions).

Actions #21

Updated by Andreas Wolf almost 17 years ago

I'm sorry, but I have not thought about this enough :( There are tables which may contain the mm-refs for more than one table and column. The patch I uploaded this evening doesn't respect this, so I created a new one which does, at least as far as I see.

Please forget all-tables.diff and only use all-tables2.diff. Have fun testing ;)

Actions #22

Updated by Andreas Wolf almost 17 years ago

"How to spam a bugtracker, part 2" ;)

Sorry, there has been another mistake in all-tables2.diff. I corrected it and hopefully v3 is bug-free now...

I already emailed Rene so I hope this will be integrated rather quick.

Actions #23

Updated by Julien Bourdin over 16 years ago

-> about the last patch

Well, what about the other side of the relation?

It seems to me that in 4.1, you can declare the fallow parameter:
'MM_opposite_field' => 'other_side_field',
which means that the uid_local and uid foreign are switch from this point of view.

So it should be tested before the swap, shouldn't it?

Actions #24

Updated by Andreas Wolf over 16 years ago

Hm, Julien, you are right, but I have to admit that I don't know how to handle this.

The problem is that if MM_opposite_field is set (name this field B here) for a field A, the uids from this field are still saved in uid_foreign. The uids from field B in table C (our opposite field/table) are saved in uid_local. When publishing a record from table C, I couldn't find any clue that I have to use uid_local instead of uid_foreign, because the opposite_field-setting is not saved in the TCA of this field.
I know about it because I have configured the TCA, but how could I detect it inside version_swap()? How and where does TYPO3 in general detect this?

Actions #25

Updated by Francois Suter over 16 years ago

I have tried the patch for dam_ttcontent and not dam_ttnews (but it should be the same problem since both are two-way MM fields). Unfortunately the patch does not work for me. It doesn't seem to have any effect :-(

Actions #26

Updated by Francois Suter over 16 years ago

Sorry about the mention of dam_ttnews (that's for related bug 3907), but the lack of solution is sadly real...

Actions #27

Updated by Ohk over 16 years ago

I see about 10 patches. I read the posts but i ask me, which patches i need?
Could anyone help me?
thanks

Actions #28

Updated by Ingo Renner about 16 years ago

Hi all,

please check whether this has been fixed with Kasper's latest changes in trunk or the upcoming 4.2 beta1. If that is the case please provide some feedback here, so that we can mark this issue as resolved.

Actions #29

Updated by Administrator Admin about 16 years ago

Issue has been fixed, confirmed to work by Francois Suter. Patch is attached to this issue including an rtf document describing what the problem was, how it was solved and also how you can make DAM compatible. There are a few places in DAM related code outside TYPO3 that needs patching, but its easy to find and do, just read that document.

Actions #30

Updated by Administrator Admin about 16 years ago

I just forgot a few details:

- The patch attached is already committed to trunk and will be in 4.2. It is uploaded for your convenience to apply to 4.1.x
- Francois suter has some working DAM modifications I'm sure he would like to share with you. Also, he has a patched 4.1.5 version

Actions #31

Updated by Francois Suter about 16 years ago

Hi all,

As Kasper announced, please find the following URLs to download the patched version of TYPO3 4.1.5, DAM 1.0.100 and DAM TV Connector 0.0.12:

http://www3.cobweb.ch/fileadmin/mmetusalem/typo3_src-4.1.5-patched.tgz
http://www3.cobweb.ch/fileadmin/mmetusalem/T3X_dam-1_0_100-z-200802011443.t3x
http://www3.cobweb.ch/fileadmin/mmetusalem/T3X_dam_tv_connector-0_0_12-z-200802011443.t3x

These are the versions that were patched by Kasper himself on our development machine.

Actions #32

Updated by Francois Suter about 16 years ago

Additionnally I am attaching to this bug report a patch that can be applied to 4.1.5. Kasper's patch was made against the trunk and does not apply properly to 4.1.x.

Actions #33

Updated by Francois Suter about 16 years ago

I will be submitting separate reports for the changes to DAM.

Actions #35

Updated by Ingo Renner about 16 years ago

fixed by Kasper in 4.2beta1

Actions #36

Updated by Michael Stucki over 10 years ago

  • Category set to Workspaces
Actions #37

Updated by Michael Stucki over 10 years ago

  • Project changed from 624 to TYPO3 Core
  • Category changed from Workspaces to Workspaces
  • Target version deleted (0)
Actions

Also available in: Atom PDF