thewca / wca-workbook-assistant

Java application for processing Excel workbooks with WCA competition results for uploading them to the WCA database
7 stars 4 forks source link

Fix for #73 + some misc stuff #91

Closed jfly closed 10 years ago

jfly commented 10 years ago

I added a TEST_SCRAMBLE_FILES variable to WorkbookAssistantFrame.java to speed up development.

I changed the event string for SCRAMBLES in WorkbookAssistantEnv.java, this should reduce activity when scrambles change. Hopefully it doesn't introduce any bugs.

The meat of this diff is the addition of a dialog to edit scrambles (accessible via an "Edit" button next to the add and remove scrambles buttons). Full disclosure: I haven't tested this very extensively (specifically, I haven't tried adding or removing scramble sources, nor have I actually checked the resulting JSON to see if it reflects the changes I've made), but what testing I have done seems to work.

One big issue: Right now, the workbook assistant refuses to put multiple groups in a Round with the same groupId (see https://github.com/cubing/wca-workbook-assistant/blob/master/src/main/java/org/worldcubeassociation/workbook/scrambles/RoundScrambles.java#L26). There's nothing in the JSON 0.2 version (see #48) that requires this, although I bet James & the website don't support groups in a round with the same groupId. The reason this is important is because the wca-workbook-assistant's enforcement that groups in a round don't have the same groupId makes moving sheets between rounds impossible in most cases (you almost always get groupId collisions!). I don't believe the correct fix is to relax the wca-workbook-assistant's requirement, I think it's a good thing to have. Instead, I think we need to expand upon the work I've done here to let people change the groupIds for sheets. That way, people will not be blocked from moving sheets to where they need them to go.

One last comment: I chose not to require that if you're moving a sheet from a round of event Foo, that you drop it into a round that is also of event Foo. I figured it's possible that people maybe generate 333 scrambles for an extra round of 333oh, and will need to move sheets between the two events. Thoughts on this are appreciated!