Modify

Opened 4 years ago

Closed 4 months ago

Last modified 4 months ago

#8781 closed defect (fixed)

Template reapplied from scratch on preview --> loss of modifications

Reported by: tools@… Owned by: rjollos
Priority: normal Component: TracTicketTemplatePlugin
Severity: normal Keywords:
Cc: Trac Release: 0.12

Description

To get the outcome:

  1. Select a component --> original template applies
  2. Type in modifications to the template
  3. Hit "Preview" --> the preview has the modifications typed in step 2; but the original template reapplies on the textarea, and we go back to step 1.

I guess it's much easier said that done, but i guess the plugin shouldn't do anything when in preview mode.

Attachments (1)

check_for_ticket_draft.patch (1.2 KB) - added by fleimgruber 4 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 follow-up: Changed 4 months ago by fleimgruber

FWIW, solved via the attached patch.

Let me know if there is a better way.

comment:2 in reply to: ↑ description Changed 4 months ago by rjollos

  • Owner changed from richard to rjollos
  • Status changed from new to accepted

Replying to tools@…:

To get the outcome:

  1. Select a component --> original template applies

Do you mean Select a ticket type?

I'm unable to reproduce the issue. Could you post your [tickettemplate] configuration?

comment:3 Changed 4 months ago by rjollos

I misunderstood the issue. I am able to reproduce now. I should have the issue fixed shortly.

comment:4 Changed 4 months ago by rjollos

In 14130:

0.9dev: Don't reload template on preview or when ticket validation fails. Refs #8781.

comment:5 in reply to: ↑ 1 ; follow-up: Changed 4 months ago by rjollos

  • Resolution set to fixed
  • Status changed from accepted to closed

Replying to fleimgruber:

Let me know if there is a better way.

The main problem with the patch is that the ticket type selection no longer works after a preview.

I believe the issue is fixed in [14130]. Would you kindly test it out and report back? I also pushed some refactoring in [14129] and another fix in [14131], so it would be a good idea to test all the functionality of the plugin, just to be sure I didn't break anything.

Changed 4 months ago by fleimgruber

comment:6 in reply to: ↑ 5 ; follow-up: Changed 4 months ago by fleimgruber

I use lazy git too much. The attached patch was an old one - this one looks better and actually works.

Replying to rjollos:

Replying to fleimgruber:

Let me know if there is a better way.

The main problem with the patch is that the ticket type selection no longer works after a preview.

Do you mean template application on ticket type change does not work anymore? I think this is reasonable behaviour in preview mode, since it would override the changes made by the user.

I believe the issue is fixed in [14130]. Would you kindly test it out and report back? I also pushed some refactoring in [14129] and another fix in [14131], so it would be a good idea to test all the functionality of the plugin, just to be sure I didn't break anything.

I will test this during our next maintenance period.

comment:7 in reply to: ↑ 6 ; follow-up: Changed 4 months ago by rjollos

Replying to fleimgruber:

Do you mean template application on ticket type change does not work anymore? I think this is reasonable behaviour in preview mode, since it would override the changes made by the user.

The edits in the description field will be overwritten anytime the ticket type is changed, regardless of preview mode or not, that is why the user is prompted with a dialog. With the changes I implemented in [14130] the behavior will be consistent regardless of whether Preview has been requested.

I know those dialogs are a bit "old-style", but that's just a pre-existing design issue with this plugin that we can improve on later.

comment:8 in reply to: ↑ 7 ; follow-up: Changed 4 months ago by fleimgruber

Replying to rjollos:

Replying to fleimgruber:

Do you mean template application on ticket type change does not work anymore? I think this is reasonable behaviour in preview mode, since it would override the changes made by the user.

The edits in the description field will be overwritten anytime the ticket type is changed, regardless of preview mode or not, that is why the user is prompted with a dialog.

Using the attached patch, the description field will not be overwritten on ticket type change when in preview mode (determined by the hackish xpath query). Note that I made a correction to the patch today.

With the changes I implemented in [14130] the behavior will be consistent regardless of whether Preview has been requested.

I know those dialogs are a bit "old-style", but that's just a pre-existing design issue with this plugin that we can improve on later.

Yes, there should be a dialog to have consistent behavior regardless of preview mode or not in the future.

Last edited 4 months ago by fleimgruber (previous) (diff)

comment:9 in reply to: ↑ 8 ; follow-up: Changed 4 months ago by rjollos

Replying to fleimgruber:

Using the attached patch, the description field will not be overwritten on ticket type change when in preview mode (determined by the hackish xpath query). Note that I made a correction to the patch today.

Yes, I understand that is how your patch works. I don't think that is the proper behavior though. I think the proper behavior is to always keep the ticket type change functional and let the dialog warn the user against overwriting the changes. The latter is how I've implemented the change in [14130].

If you don't like the behavior I've implemented and choose to continue using your patch, you should check for 'preview' in req.args rather than trying to inspect the Genshi stream.

comment:10 in reply to: ↑ 9 Changed 4 months ago by fleimgruber

Replying to rjollos:

Replying to fleimgruber:

Using the attached patch, the description field will not be overwritten on ticket type change when in preview mode (determined by the hackish xpath query). Note that I made a correction to the patch today.

Yes, I understand that is how your patch works. I don't think that is the proper behavior though. I think the proper behavior is to always keep the ticket type change functional and let the dialog warn the user against overwriting the changes. The latter is how I've implemented the change in [14130].

[14130] is indeed the better solution. I was referring to the viewpoint of my users which were more concerned about loosing their edits than the reapplication of the templates recently, thus the ad-hoc dirty patch.

If you don't like the behavior I've implemented and choose to continue using your patch, you should check for 'preview' in req.args rather than trying to inspect the Genshi stream.

As I said, I am planning to test [14130] in the future. Thanks for the feedback, checking req.args is much more straight forward than my approach. This being my first interaction with trac at this level, I was just stumbling around grasping for straws and got hold of this one...

Add Comment

Modify Ticket

Action
as closed The owner will remain rjollos.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.