-
-
Notifications
You must be signed in to change notification settings - Fork 841
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
Track watch activity for scenes. #3055
Track watch activity for scenes. #3055
Conversation
My latest push contains changes to address feature #633. This pull request now tracks watchTime and viewCount and provides options to filter and sort by those new fields. A config option was also created to allow users to set an ignore interval to delay tracking until a set amount of time has passed. There is still a bug with sorting by View Count that I need to address which I will look into tomorrow. Some debug code also needs to be cleaned out in typescript files. |
If I interpret your Code correctly, this would make it relatively easy to add a filter for already started videos, as well as a sorting by 'last watched'. I think both would be a great addition to the project, so I'd be thankful if you consider adding those to the request (or create a separate one of course). Sorry about the duplicates. Github just broke for some reason, gave me back timeouts but obviously still posted them after... |
I see you asked the same comment 3 times. Can you please remove 2 of them to remove unnecessary bloat from this pull request? To answer the question, this change already has a sort option to sort by recently viewed scenes. I didn’t add a filter for filtering on watch progress but that should be painless to do so I can include it. |
Awesome, thank you! |
Hey, this looks great! Sorry, I'm new to Stash but will it be possible with this addition of this recently played data for me to import the played data I have stored in a different organizing program? I'm still working on the necessary plugin but I would like to transfer data such as Scene X - played 21.34 Aug 12 2022, 17.59 Feb 14 2022, 18.43 Dec 25 2021 etc. Or at least could this pull be set up to enable such info (I guess it's multiple dates played attached to a scene) to be included and open to plugins in the future? Thanks much! |
You should be able to update the activity data via graphql mutations so a script can be written to import your data. However, to keep things simple, this change does not track all the dates an event occurred on. It only keeps track of the date of the most recent activity and increments the play count and duration accordingly. Storing this info by the date of each occurrence is something I'm looking into, but I likely won't have time to get in with this change since it would touch code that I need more time to become more familiar with. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stuff I found while testing:
- in the settings, disable the interval setting instead of hiding if activity tracking is disabled.
- set track activity setting to true by default
- we should provide ways to set/reset
resume_time
,play_duration
andplay_count
, even if these aren't used in the UI - there's a stale cache issue when clicking the scenes button from the player. I had to refresh before the scene displayed the correct resume time in the scenes list
- going to the next scene in the playlist plays from the resume time, which I've not sure should be the intended behaviour
- export/import needs to include the new fields
pkg/sqlite/scene.go
Outdated
if err := qb.tableMgr.checkIDExists(ctx, id); err != nil { | ||
return 0, err | ||
} | ||
|
||
if err := qb.tableMgr.updateByID(ctx, id, goqu.Record{ | ||
"resume_time": resumeTime, | ||
}); err != nil { | ||
return 0, err | ||
} | ||
|
||
if err := qb.tableMgr.updateByID(ctx, id, goqu.Record{ | ||
"play_duration": goqu.L("play_duration + ?", playDuration), | ||
}); err != nil { | ||
return 0, err | ||
} | ||
|
||
if err := qb.tableMgr.updateByID(ctx, id, goqu.Record{ | ||
"play_count": goqu.L("play_count + 1"), | ||
}); err != nil { | ||
return 0, err | ||
} | ||
|
||
if err := qb.tableMgr.updateByID(ctx, id, goqu.Record{ | ||
"last_played_at": time.Now(), | ||
}); err != nil { | ||
return 0, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should only call updateByID
once, using a single combined goqu.Record
.
@@ -0,0 +1,4 @@ | |||
ALTER TABLE `scenes` ADD COLUMN `resume_time` float not null default 0; | |||
ALTER TABLE `scenes` ADD COLUMN `last_played_at` datetime not null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This causes an error during migration for existing systems as there is no default and is not null
. It should accept and default to a null value.
pkg/sqlite/scene.go
Outdated
StudioID null.Int `db:"studio_id,omitempty"` | ||
CreatedAt models.SQLiteTimestamp `db:"created_at"` | ||
UpdatedAt models.SQLiteTimestamp `db:"updated_at"` | ||
LastPlayedAt models.SQLiteTimestamp `db:"last_played_at"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be models.NullSQLiteTimestamp
.
pkg/models/model_scene.go
Outdated
@@ -35,6 +35,11 @@ type Scene struct { | |||
CreatedAt time.Time `json:"created_at"` | |||
UpdatedAt time.Time `json:"updated_at"` | |||
|
|||
LastPlayedAt time.Time `json:"last_played_at"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be *time.Time
, where nil
indicates never played.
alwaysStartFromBeginning | ||
trackActivity | ||
ignoreInterval |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that these are exclusively used in the UI, we should consider moving them into the UI properties, instead of adding more backend config handles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the UI properties file you want this to be moved to?
I'm just now seeing your feedback. Thanks for going through this. I'll start working my way through your comments soon. |
I don't think the caching issue you're seeing is new to this change. I've been noticing that Stash doesn't always properly refresh when navigating to a page that has been visited earlier. I'm not yet clear on how this is handled in React/Stash. |
Can you please merge or rebase against the |
I should be able to do this tomorrow. I’ve been busier than usual lately, but things should start to slow down tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cleaned up the backend code a bit and added unit tests.
I'm finding that the resume time gets reset when I navigate to a scene page without playing. I assume this is not expected behaviour?
Currently missing code to export the new fields.
I'm getting database locked errors when incrementing the play count. I'm going to investigate this issue separately, so it can stay as a known issue for the moment.
This pull request adds activity-tracking support for videos. Stash will now remember where you left off in a video and will resume from that position when you return to the video. Scene cards will now include a progress bar to show users how far into each video they are. Users can also now sort scenes by recently played to see what video they last watched.
I added a config option to tell Stash to always start a video from the beginning for users who might not want resume functionality.
Fixes #2769
Fixes #633