Skip to content
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

Draft: Show indepth exercise history changes #1082

Conversation

ImTheTom
Copy link
Contributor

@ImTheTom ImTheTom commented Jun 30, 2022

Proposed Changes

  • Adds a new page to show in depth exercise cahnges

Please check that the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features) TODO
  • Added yourself to AUTHORS.rst

Other questions

  • Hey just placing this up to show what I've got if you have any concerns. At the moment the activty streams data column holds the associated history ID.
  • Still left to do is to look into showing off create and delete changes and handling their reverts.
  • Also need to make the code more defensive / write tests.
  • Might also look into using the revert_url on history objects.
    When trying to access the revert_url on the history object at the moment it displays
LookupError: No installed app with label 'admin'.

Here's a video of it currently.
Shows updating exercise object id 1 author and description and then only reverting the license author.

2022-06-30.12-32-51.mp4

@rolandgeider
Copy link
Member

Awesome! I'll probably only be able to look at this in two weeks (there's some birthdays coming and next week we're going to airbeat one 😍)

@ImTheTom
Copy link
Contributor Author

ImTheTom commented Jul 1, 2022

Yeah all good. Have fun. :)

@ImTheTom
Copy link
Contributor Author

ImTheTom commented Jul 5, 2022

Hey, was planning on adding support for created, updated and deleted exercises on the this page an option to revert each action.

Just realised when you delete a model it also deletes all the actstream actions for the model. Looking at their closed issues it seems like they don't want to support an option to keep them when you delete the model as well. So, I would have to swap around the logic to have the history models know about the activity streams.

Just checking with you if you think having the option to revert deletions is worth it as well, guess we wouldn't know who deleted it as the activity streams would be gone, but having an option to add it back would be nice.

@rolandgeider
Copy link
Member

mhh

I would say it is OK if we lose the information when an exercise is deleted. This will be only done by admins and we don't need to document everything, we're not a bank

@ImTheTom
Copy link
Contributor Author

Yeah makes sense, sounds good. I'll clean it up and make sure everything works tomorrow.

@rolandgeider rolandgeider linked an issue Aug 19, 2022 that may be closed by this pull request
@rolandgeider
Copy link
Member

I simplified the logic here so that we don't need to explicitly save the history ID. I'm trying to finish everything left on the crowdsourcing branch so we can finally finally realease that stuff. In any case thanks again for the PR

@rolandgeider rolandgeider merged commit fa0607f into wger-project:feature/exercise-crowdsourcing Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show history with changes to exercises data
2 participants