New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the Flag's target management #7

Closed
acotiuga opened this Issue Jun 5, 2018 · 0 comments

Comments

Projects
None yet
1 participant
@acotiuga
Copy link
Collaborator

acotiuga commented Jun 5, 2018

The target property from ForumCode.FlagClass is a concatenation of 2 strings: topic's full name and answer's id. The result is "target=${escapetool.url($doc.fullName)}:${topicAnswerID}". Later on, this property is split in multiple places and the separator is considered to be the colon char (:). So, besides this string composing and splitting play, there could be cases when : is present more than once in any of those 2 string and the results could be unexpected (missing string parts). Also the url escaping is not very clear with the current approach.

The plan is to add a new anchor property in the ForumCode.FlagClass to store the answer's id, keep the target for the topic's full name and provide a migrator from v2.4.2 to 2.5 to fix the old flags.

@acotiuga acotiuga added this to the 2.5 milestone Jun 5, 2018

@acotiuga acotiuga self-assigned this Jun 5, 2018

acotiuga added a commit that referenced this issue Jun 5, 2018

acotiuga added a commit that referenced this issue Jun 5, 2018

Fix the Flag's target management #7
* strange leftover from https://jira.xwiki.org/browse/XAFORUM-122. For some reason, all the changes from that issue were moved to the PRO version, except those from the custom fields.

acotiuga added a commit that referenced this issue Jun 5, 2018

Fix the Flag's target management #7
* extract the common code to obtain the flag link
* reuse css class and remove rules that don't make sense anymore
* introduce the new "anchor" property
* use 2 separated properties, "target" and "anchor" instead of havin one "target" with ":" as separator

acotiuga added a commit that referenced this issue Jun 5, 2018

Fix the Flag's target management #7
* add the migration script to split the 'target' value in 'target' and 'anchor'.

acotiuga added a commit that referenced this issue Jun 6, 2018

Fix the Flag's target management #7
* add anchor to flag url only when it exists

acotiuga added a commit that referenced this issue Jun 8, 2018

Fix the Flag's target management #7
* add functional tests

acotiuga added a commit that referenced this issue Jun 8, 2018

Fix the Flag's target management #7
* clean functional tests

@acotiuga acotiuga closed this Jun 8, 2018

acotiuga added a commit that referenced this issue Jun 18, 2018

Fix the Flag's target management #7
* the flagged page should be accessed in the same window
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment