[Unicode]   Common Locale Data Repository : Bug Tracking Home | Site Map | Search
 
Modify

CLDR Ticket #11056(closed survey: fixed)

Opened 4 months ago

Last modified 2 months ago

Importing Old votes process

Reported by: kristi Owned by: tbishop
Component: survey Data Locale:
Phase: dsub Review: kristi
Weeks: Data Xpath:
Xref:

Description (last modified by kristi) (diff)

Discussion from 4/18 TC meeting

  1. Modify the current import prompt at first log-in to be automatic without requiring user action

winning (= votes for what won)

  1. Add a first log-in message that indicates "Your old winning votes for locales de-de; de-CH have been imported". "OK" (i.e. require no action by user)

(Note that some users work on more than one locale, so we should take that into consideration.)

  1. Losing votes can be viewed and selected for import from the Setting gear-->under "My Votes" add "Import my old losing votes"
  2. The Losing votes import should not have Select all (the current functionality include this and should be removed)

Once the tools implementation is done, we should update the translation guide.

Note: there is a Tool change agreement needed that supports this documentation, which is to Remove the Select All option for import of Losing votes.

Attachments

Import-new section in Survey Tool guide.docx (235.2 KB) - added by kristi 4 months ago.
Import-New automated Winning and manual losing.docx (118.8 KB) - added by kristi 4 months ago.
New TO BE process
Import-New automated Winning and manual losing-Edited by Fredrik.docx (118.2 KB) - added by kristi 4 months ago.
Updated with Fredrik's review and edits

Change History

Changed 4 months ago by kristi

comment:1 Changed 4 months ago by kristi

  • Description modified (diff)
  • Summary changed from How to Import Survey tool guide to Importing Old votes process

Changed 4 months ago by kristi

New TO BE process

comment:2 Changed 4 months ago by kristi

  • Description modified (diff)

comment:3 Changed 4 months ago by kristi

  • Owner changed from anybody to frontend
  • Status changed from new to accepted
  • Milestone changed from UNSCH to 33.1

comment:4 Changed 4 months ago by kristi

  • Keywords STP1 added

comment:5 follow-up: ↓ 6 Changed 4 months ago by fredrik

Important note: This process should only apply to VETTER and STREET users.
Others, like TC, may have applied 20-value votes in previous sessions that should not be automatically be thrown in.

comment:6 in reply to: ↑ 5 Changed 4 months ago by fredrik

winning (= votes for what won)

I suggest changing it to
winning (= votes that match the value used in the most recent release)
... or words to that effect. A vote may have been registered as winning during the most recent effort, but later overridden or changed by TC and not longer winning.

Changed 4 months ago by kristi

Updated with Fredrik's review and edits

comment:7 Changed 4 months ago by kristi

BTW "Winning" could have the "Contributed" value if a locale or the field requires more than 4 votes, but if there's only one vetter with 4 vote power.

comment:8 Changed 4 months ago by tbishop

Based on my meeting with Mark yesterday, I'm to make this ticket my top priority (as one of the four currently marked STP1). Questions:

  • Is it time to create a new branch for this?
  • Will more than one of us be editing the code?
  • Do we have any unit tests covering any of survey.js yet?
  • Is part of the plan to replace the current gear-menu item "Import Old Votes" with a new item "Import My Old Losing Votes"?
Last edited 4 months ago by tbishop (previous) (diff)

comment:9 Changed 4 months ago by tbishop

Proposed plan:

  1. Set up data on localhost in a state such that I have old winning and losing votes not yet imported, with the ability to return to that state repeatedly for interactive testing.
  1. Move the block of code in SurveyAjax.java starting with "} else if (what.equals("oldvotes")) {" (222 lines, in processRequest which is currently 1162 lines long) to a new function.
  1. Create a unit test for the new function.
  1. Refactor the new function, and add to it the abilities to import all old winning votes automatically, and old losing votes interactively (no "select all").
  1. Add unit tests for the new abilities of the new function.
  1. Revise the frontend to support the new abilities while temporarily also still supporting the old ability, based on a boolean setting to switch between new/old abilities.
  1. After thorough testing of the new abilities, possibly remove the old ability.

comment:10 Changed 4 months ago by emmons

  • Priority changed from assess to critical
  • Type changed from unknown to survey
  • Component changed from unknown to survey
  • Milestone changed from 33.1 to 34

comment:11 Changed 4 months ago by tbishop

Step 1 of the above plan is done: Set up data on localhost in a state such that I have old winning and losing votes not yet imported, with the ability to return to that state repeatedly for interactive testing. Using derby, the db can be backed up (and restored) by copying from (and to) the folder workspace/.metadata/.plugins/org.eclipse.wst.server.core/tmp0/cldr/cldrdb. The ST built-in SQL console cldr-apps/survey?sql=CLDR_VAP is handy for seeing what's currently in the tables. After voting both as admin and as a vetter, the db has 4 old votes for the vetter, of which 2 are winning and 2 are losing. Import Old Votes appears to work fine, using table CLDR_VOTE_VALUE_33 and creating table CLDR_VOTE_VALUE_34.

Next I'll create branches/tbishop/t11056 and a new function in SurveyAjax.java.

comment:12 Changed 4 months ago by tbishop

  • Owner changed from frontend to tbishop

comment:13 Changed 4 months ago by tbishop

I think I've finished implementing the changes for this ticket, except for unit tests, and possibly one detail.

I propose to postpone finishing new unit tests for this ticket. Unit tests are time-consuming to write for large modules that don’t already have unit tests, and dependencies are hard to set up in the current test framework. For now, other tickets may be higher priorities. I still recommend that the implementation of more unit tests should be a fairly high priority.

The one detail remaining is this: after old winning votes are automatically imported, the user is shown a dialog, "n old winning votes were automatically imported", "OK". Clicking OK simply dismisses the dialog. There may still be old losing votes that haven't been imported; if so, the user isn't shown a "reminder" dialog for that until the next time they log in. If that "reminder" dialog is going to be shown at all, it should probably be immediately after the "automatically imported" dialog is dismissed.

Question: do we still want the "reminder" dialog for old losing votes? If so, I'll make it appear immediately after dismissing the "automatically imported" dialog. If not, I'll remove the "reminder" dialog code.

comment:14 Changed 3 months ago by tbishop

By email, the answer to "do we still want the reminder dialog for old losing votes" was unanimously no. Therefore I've deleted the reminder code.

I've made a video of testing on localhost. It looks fine to me:

http://wenlininstitute.org/cldr_movie/auto_import_old_votes_edited.mp4

Per email and phone discussion I merged into trunk and did smoke test. Steven explained how to find "old votes" for version 33 to import into version 34, and log in as the appropriate users.

Last edited 3 months ago by tbishop (previous) (diff)

comment:15 Changed 3 months ago by tbishop

  • Status changed from accepted to reviewing
  • Review set to kristi

comment:16 Changed 3 months ago by tbishop

I committed the changes to trunk, and ran smoke test, imported old votes for two users. For the first user (tarita, user #139), auto-imported 26 winning, leaving 4 losing; imported one losing vote manually. As intended, the "choose all" button no longer appears for importing manually. For the second user (teura, user #138), auto-imported 8 winning, leaving 2 losing; imported one losing vote manually. The "choose none" button still appears and works correctly. The imports appear to be successful. Imported votes show as new rows in cldr_vote_value_34_beta. Movies:

http://wenlininstitute.org/cldr_movie/auto_import_old_votes_smoketest1.mp4

http://wenlininstitute.org/cldr_movie/auto_import_old_votes_smoketest2.mp4

Committed to trunk as revision 14042 (<http://unicode.org/cldr/trac/changeset/14042>)

Last edited 3 months ago by tbishop (previous) (diff)

comment:17 Changed 3 months ago by kristi

  • Review changed from kristi to srl

Steven could you please review this from Code point of view?

Once code review is done, you can assign back to me.
We'll perform the actual import in production during shakedown.

comment:18 Changed 3 months ago by tbishop

I just noticed in the comments above:

"Important note: This process should only apply to VETTER and STREET users."

Sorry I missed that earlier. I haven't restricted the process in that way yet! Per email from Mark, something more should happen with TC users, will be another ticket, for now no restriction here on type of user.

Also, about recent cldr-dev email: the whole "reminder" feature was removed by the change already made for ticket 11056 - as agreed on cldr-dev. The current dialog has only an OK button. (v_oldvote_remind_msg was replaced by v_oldvote_auto_msg in <https://unicode.org/cldr/trac/changeset/14042>.) OK as-is?

My understanding is that there's no more to be done here now other than review.

Last edited 3 months ago by tbishop (previous) (diff)

comment:19 Changed 3 months ago by srl

  • Status changed from reviewing to reviewfeedback

code looks great. Please assign reviewer to Kristi for functional review.

comment:20 Changed 2 months ago by tbishop

  • Review changed from srl to kristi

comment:21 Changed 2 months ago by kristi

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

Functionality was tested on smoketest and there's been no additional feedback from actual usage in production.

View

Add a comment

Modify Ticket

Action
as closed
Next status will be 'new'
Next status will be 'closed'
Author


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

 
Note: See TracTickets for help on using tickets.