Skip to content
This repository has been archived by the owner on Oct 19, 2021. It is now read-only.

In Progress: Delete Results from History #12

Closed
gssjr opened this issue Jan 7, 2015 · 13 comments
Closed

In Progress: Delete Results from History #12

gssjr opened this issue Jan 7, 2015 · 13 comments

Comments

@gssjr
Copy link

gssjr commented Jan 7, 2015

I'm opening this thread to start communication on where you would like to see this feature be taken (delete results from History).

I have nearly all the back-end stuff done but need to work on the UI/UX ( here is mock-up https://cloudup.com/c8cpSYkSnW6 ). I'm imagining that there will be an edit "mode" such that when the user clicks "Edit History" the checkboxes and "Delete Selected" button will appear. The "Edit History" button will have to change to say something like "Cancel Edit."

EDIT: I was thinking of an alternative approach inspired by Google Inbox: When the user hovers over the player hero's icon it will switch to a checkbox. When the user checks one or more rows it will display the bulk edit controls (e.g. Delete).

For now, Arena results cannot be deleted since there are further considerations. I didn't dig into the result submission code yet, but I'm guessing the arena gets created after the first arena result is created. Not sure if there can be an empty arena. I noticed in the arena view it calls arena.results.created_at which will error if the last result in an arena is deleted. But that's something we can discuss how to handle. I will have the checkboxes disabled on Arena results for now.

@bergren2
Copy link

bergren2 commented Jan 7, 2015

Nice UI. This other issue is related, albeit with slightly different controls: stevschmid/track-o-bot#65

Are we worried about users deleting unfavorable matches? (so their match history looks better than it actually was) Or is that beyond the scope of Track-o-Bot?

@gssjr
Copy link
Author

gssjr commented Jan 7, 2015

I don't think it really matters. The user in only cheating himself or herself if they do that. Currently, there are no aggregate statistics amongst all users, but this could skew the results if such a player-wide statistics feature was added (similar to Hearthstats). I'd expect most people wouldn't be so disillusioned as to fake their results, especially there's no social component to trackobot. More so, I doubt enough people would do it to affect any aggregate statistics.

@Baco9
Copy link

Baco9 commented Feb 14, 2015

Is this still being developed? Can I help?
I'm really waiting for this feature ;)

@gssjr
Copy link
Author

gssjr commented Feb 14, 2015

I got pretty busy after starting it so haven't finished yet. But I haven't forgotten. It's nearly done just need to figure out a few bugs.

@stevschmid
Copy link
Owner

Take your time!

@gssjr
Copy link
Author

gssjr commented Feb 18, 2015

@stevschmid What do you think of this UI? http://take.ms/NmpfP

I'm wondering if we should make these soft deletes so data is recoverable? Do you have a preference?

I still need to figure out how to handle arena results since the results belong to an arena run. I'm not sure what types of scenarios are existing for trackobot users that have issues with arena results that are causing them to want to delete them; that may help in deciding how to implement the delete feature. Currently, I'd have to update the timestamp behavior in arena list since it only uses the last result date, which would throw an error in the list view after deleting the final result in a run. Also, I would probably have to add the option to delete a whole arena run itself. There also exists the issue if trackobot thinks an Arena run is complete, we'd have to re-open the run if some results are deleted. Do you know if a user has ever reported a single Arena in Hearthstone accidentally being tracked as two runs in trackobot?

Once we square out some of the details for arenas, I can implement that as well. However, we can push the deletion of non-arena events earlier since in both the UI and model I prevent deletion of arena results.

@stevschmid
Copy link
Owner

Nice work! Looks pretty cool!

Persistence: No recovery for now.

Arena: The ability to delete an arena run would be cool as well. As for the deletion of results which belong to an arena: Not sure what you mean by the timestamp behavior. Shouldn't it be enough to just delete an associated result? I would keep it really simple because it will be used rarely.

@gssjr
Copy link
Author

gssjr commented Feb 18, 2015

It was kind of a trivial comment in retrospect, but just to clarify:

The Arena index view has: <td class="timestamp"><%= local_time_ago(arena.results.order('created_at DESC').first.created_at) %></td> which would give an error: NoMethodError: undefined method 'created_at' for nil:NilClass

So, it'd just be a simple adjustment to account for Arena runs with no results.

@stevschmid
Copy link
Owner

Ah yes. The deletion of the last result of an arena run should also delete the run.

@stevschmid
Copy link
Owner

Any updates on this?

@gssjr
Copy link
Author

gssjr commented Apr 7, 2015

Sorry, been lazy. I can send a pull request your way if you want to implement what I have so far, or work off of what I have.

Todo:

  • Implement Arena deletions
  • Create front-end javascript/integration tests

@stevschmid
Copy link
Owner

@gssjr Please create a PR with what you have so far. Maybe somebody else has the time and passion to tackle it.

@stevschmid
Copy link
Owner

Upcoming version will have this feature. Thanks @gssjr.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants