BUGFIX: fix re-fetching of conflicting users having a crash if the user in question was banned instead of deleted

CHANGELOG.txt

@@ -393,3 +393,16 @@ v0.6.22
 
 - Add background scraping jobs for Bookmarks and logged-in user's User Feed
 - Manually rescraping a User Feed now re-downloads user's profile and banner images as well
+
+v0.7.0
+------
+
+- PKG: Type definitions are now in `pkg/persistence` instead of `pkg/scraper`
+- BUGFIX: fix notifications page not having user's "likes" filled out on tweets
+- Compound notifications now tell you how many tweets were "liked", how many users followed you, etc
+
+v0.7.1
+------
+
+- BUGFIX: fix banned users getting their user info (followers count, profile image, display name) getting erased
+- PKG: webserver is now a public package

cmd/compile.sh

@@ -4,7 +4,7 @@ set -x
 set -e
 
 # General build flags
-FLAGS="-s -w -X gitlab.com/offline-twitter/twitter_offline_engine/internal/webserver.use_embedded=true"
+FLAGS="-s -w -X gitlab.com/offline-twitter/twitter_offline_engine/pkg/webserver.use_embedded=true"
 
 # Check for the `--static` flag and consume it
 USE_STATIC=false

cmd/tests.sh

@@ -349,6 +349,11 @@ test $(sqlite3 twitter.db "select is_stub, user_id = 0x4000000000000000 from twe
 tw fetch_user nancytracker
 test "$(sqlite3 twitter.db "select is_banned from users where handle='nancytracker'")" = "1"
 
+# Fetch a thread with a banned user in it; test no-clobbering of the existing user data
+sqlite3 twitter.db "insert into users (id, handle, display_name, profile_image_url, banner_image_local_path, followers_count) values (1595500307374829568, 'spandrell3', 'Spandrell', 'Profile URL', 'Banner Local', 10)"
+tw fetch_tweet https://twitter.com/spandrell3/status/1709580398026805736
+test "$(sqlite3 twitter.db "select display_name, profile_image_url, banner_image_local_path, followers_count, is_banned from users where handle='spandrell3'")" = "Spandrell|Profile URL|Banner Local|10|1"
+
 
 # Fetch a user with "600x200" banner image
 tw fetch_user AlexKoppelman  # This is probably kind of a flimsy test

cmd/twitter/helpers.go

@@ -112,6 +112,9 @@ func full_save_tweet_trove(trove TweetTrove) {
 			// Mark them as deleted.
 			// Handle and display name won't be updated if the user exists.
 			updated_user = User{ID: u_id, DisplayName: "<Unknown User>", Handle: "<UNKNOWN USER>", IsDeleted: true}
+		} else if errors.Is(err, scraper.ErrUserIsBanned) {
+			// Mark them as banned (also won't clobber handle and display name)
+			updated_user = User{ID: u_id, DisplayName: "<Unknown User>", Handle: "<UNKNOWN USER>", IsBanned: true}
 		} else if err != nil {
 			panic(fmt.Errorf("error scraping conflicting user (ID %d): %w", u_id, err))
 		}

cmd/twitter/main.go

@@ -14,9 +14,9 @@ import (
 	"syscall"
 	"time"
 
-	"gitlab.com/offline-twitter/twitter_offline_engine/internal/webserver"
 	. "gitlab.com/offline-twitter/twitter_offline_engine/pkg/persistence"
 	"gitlab.com/offline-twitter/twitter_offline_engine/pkg/scraper"
+	"gitlab.com/offline-twitter/twitter_offline_engine/pkg/webserver"
 )
 
 // Global variable referencing the open data profile

cmd/windows-compile.sh

@@ -14,12 +14,12 @@ if [[ -z "$1" ]]; then
 fi
 
 # Always use static build for windows
-FLAGS="-s -w -X gitlab.com/offline-twitter/twitter_offline_engine/internal/webserver.use_embedded=true"
+FLAGS="-s -w -X gitlab.com/offline-twitter/twitter_offline_engine/pkg/webserver.use_embedded=true"
 SPECIAL_FLAGS_FOR_STATIC_BUILD="-linkmode=external -extldflags=-static"
 
 # Add an application icon using `windres` and `.syso`
 # The `.syso` should be picked up automatically, since it's in the same directory we run `go build` from`
-echo '1 ICON "../internal/webserver/static/twitter.ico"' > /tmp/icon.rc
+echo '1 ICON "../pkg/webserver/static/twitter.ico"' > /tmp/icon.rc
 x86_64-w64-mingw32-windres /tmp/icon.rc -o icon.syso
 
 go build -ldflags="$FLAGS $SPECIAL_FLAGS_FOR_STATIC_BUILD -X main.version_string=$1" -o twitter.exe ./twitter

doc/TODO.txt

@@ -136,9 +136,6 @@ TOOD: notifications-duplicate-bug (#scraper, #bug)
 TODO: dm-reaccs-popup-list (#dms, #webserver, #reaccs)
 - on-click popup showing who sent which reacc
 
-TODO: detect-banned-users-when-scraping-tweets (#scraper, #bug)
-- banned users aren't detected when scraping tweets
-
 TODO: sqlite-query-optimizing
 - https://sqlite.org/cli.html#index_recommendations_sqlite_expert_
 - https://sqlite.org/eqp.html
@@ -150,3 +147,8 @@ TODO: deprecated-offline-follows
 - Offline Follows doesn't do much userful anymore
 
 TODO: offline-timeline-vs-user-timeline
+
+TODO: go-stdlib-path-vs-filepath (#code-quality)
+
+TODO: show-retweet-icon-in-ui (#webserver)
+- The UI should show whether the active user has retweeted the tweet, same as with "likes"

doc/tasks/bash-completions.todo

@@ -1,3 +1,9 @@
 Bash completions
 
 https://opensource.com/article/18/3/creating-bash-completion-script
+
+
+Enable with:
+```
+complete -C 'go run ./twitter autocomplete "$@"' twitter
+```

doc/tasks/notifications-mark-as-read.todo

@@ -1,8 +0,0 @@
-Notifications mark as read
-
-- it uses cursors.  so, to mark notifications as read:
-	- scrape notifications
-	- extract top cursor
-	- use that as the cursor value to mark as read
-
-DAABDAABCgABFDzeDIfVUAIIAAIAAAABCAADiJONcggABOxI6DUACwACAAAAC0FaSHRBaThqLUN3CAADyS991gAA

pkg/persistence/notification_queries_test.go

@@ -70,7 +70,7 @@ func TestLikesNotificationWithBothTweetsAndRetweets(t *testing.T) {
 
 	// Now the user "likes" another Tweet
 	new_tweet := create_dummy_tweet()
-	profile.SaveTweet(new_tweet)
+	require.NoError(t, profile.SaveTweet(new_tweet))
 	n.ActionTweetID = new_tweet.ID
 	n.ActionRetweetID = TweetID(0)
 	n.TweetIDs = append(n.TweetIDs, new_tweet.ID)

pkg/scraper/api_types_v2.go

@@ -557,6 +557,8 @@ func (e APIV2Entry) ToTweetTrove() TweetTrove {
 				panic(err)
 			}
 
+			// We don't have any user info yet.  We may be able to reconstruct some later with reply-joining
+			// But for now, it's just "Unknown User"
 			fake_user := GetUnknownUser()
 			ret.Users[fake_user.ID] = fake_user
 			parsed_tombstone_tweet.UserID = fake_user.ID
@@ -784,8 +786,10 @@ func (api_response APIV2Response) ToTweetTrove() (TweetTrove, error) {
 		if replied_tweet.UserID == 0 || replied_tweet.UserID == GetUnknownUser().ID {
 			replied_tweet.UserID = tweet.InReplyToUserID
 			if replied_tweet.UserID == 0 || replied_tweet.UserID == GetUnknownUser().ID {
+				// We know absolutely nothing about them; can't determine a UserID or handle
+				// Create a dummy user just so the Tweet will have a non-0 user ID.
 				fake_user := GetUnknownUser()
-				ret.Users[fake_user.ID] = fake_user
+				ret.Users[fake_user.ID] = fake_user // Make sure the dummy user appears in the Trove
 				replied_tweet.UserID = fake_user.ID
 			}
 		} // replied_tweet.UserID should now be a real UserID
@@ -797,6 +801,11 @@ func (api_response APIV2Response) ToTweetTrove() (TweetTrove, error) {
 		if existing_user.Handle == "" {
 			existing_user.Handle = tweet.InReplyToUserHandle
 		}
+		// If the replied tweet is a "user was suspended" tombstone, it can be inferred that the
+		// user must be suspended
+		if replied_tweet.TombstoneType == "suspended" {
+			existing_user.IsBanned = true
+		}
 		ret.Users[replied_tweet.UserID] = existing_user
 		ret.TombstoneUsers = append(ret.TombstoneUsers, existing_user.Handle)
 

pkg/scraper/api_types_v2_test.go

@@ -1013,3 +1013,35 @@ func TestNoFailOnComposerEntryInRegularThread(t *testing.T) {
 
 	assert.Len(trove.Tweets, 3)
 }
+
+// Test a thread with a banned user
+func TestParseTweetThreadWithBannedUser(t *testing.T) {
+	assert := assert.New(t)
+	require := require.New(t)
+	data, err := os.ReadFile("test_responses/api_v2/tweet_from_banned_user.json")
+	require.NoError(err)
+	var api_response APIV2Response
+	err = json.Unmarshal(data, &api_response)
+	require.NoError(err)
+
+	trove, err := api_response.ToTweetTrove()
+	require.NoError(err)
+
+	assert.Len(trove.Tweets, 6)
+	banned_tweet1, is_ok := trove.Tweets[1709579269247234284]
+	assert.True(is_ok)
+	assert.True(banned_tweet1.IsStub)
+	assert.Equal(banned_tweet1.UserID, UserID(1595500307374829568))
+	assert.Equal(banned_tweet1.TombstoneType, "suspended")
+
+	banned_tweet2, is_ok := trove.Tweets[1709580398026805736]
+	assert.True(is_ok)
+	assert.True(banned_tweet2.IsStub)
+	assert.Equal(banned_tweet2.UserID, UserID(1595500307374829568))
+	assert.Equal(banned_tweet2.TombstoneType, "suspended")
+
+	banned_user, is_ok := trove.Users[1595500307374829568]
+	assert.True(is_ok)
+	assert.Equal(banned_user.Handle, UserHandle("spandrell3"))
+	assert.True(banned_user.IsBanned)
+}

pkg/webserver/handler_messages_test.go

@@ -11,8 +11,8 @@ import (
 	"github.com/stretchr/testify/require"
 	"golang.org/x/net/html"
 
-	"gitlab.com/offline-twitter/twitter_offline_engine/internal/webserver"
 	. "gitlab.com/offline-twitter/twitter_offline_engine/pkg/persistence"
+	"gitlab.com/offline-twitter/twitter_offline_engine/pkg/webserver"
 )
 
 func TestMessagesIndexPageRequiresActiveUser(t *testing.T) {

pkg/webserver/server_test.go

@@ -9,8 +9,8 @@ import (
 	"github.com/andybalholm/cascadia"
 	"github.com/stretchr/testify/require"
 
-	"gitlab.com/offline-twitter/twitter_offline_engine/internal/webserver"
 	. "gitlab.com/offline-twitter/twitter_offline_engine/pkg/persistence"
+	"gitlab.com/offline-twitter/twitter_offline_engine/pkg/webserver"
 )
 
 type CapturingWriter struct {

pkg/webserver/temp_utils.go

@@ -20,8 +20,11 @@ func (app *Application) full_save_tweet_trove(trove TweetTrove) {
 		updated_user, err := scraper.GetUserByID(u_id)
 		if errors.Is(err, scraper.ErrDoesntExist) {
 			// Mark them as deleted.
-			// Handle and display name won't be updated if the user exists.
+			// Handle and display name won't be clobbered if the user exists.
 			updated_user = User{ID: u_id, DisplayName: "<Unknown User>", Handle: "<UNKNOWN USER>", IsDeleted: true}
+		} else if errors.Is(err, scraper.ErrUserIsBanned) {
+			// Mark them as banned (also won't clobber handle and display name)
+			updated_user = User{ID: u_id, DisplayName: "<Unknown User>", Handle: "<UNKNOWN USER>", IsBanned: true}
 		} else if err != nil {
 			panic(fmt.Errorf("error scraping conflicting user (ID %d): %w", u_id, err))
 		}