metabolist / metatext

A free, open-source iOS Mastodon client.
https://metabolist.org/metatext
GNU General Public License v3.0
727 stars 102 forks source link

"Duplicate Record" modal appeared while viewing an Announcement #228

Open alexbbrown opened 2 years ago

alexbbrown commented 2 years ago

Describe the bug

Saw Announcement icon (1). Tapped it. Saw "Duplicate Record" modal with OK. Tapped it.

Closed it. pulled to refresh the announcement. Saw "Duplicate Record" again.

To Reproduce Steps to reproduce the behavior:

  1. I have not been able to reproduce

Expected behavior

Modals are appropriate in limited circumstances 1) When it provides critical information for the user or developer to debug an issue 2) When the user must make a choice between irreversible decisions 3) When a user initiated action fails in a way the user needs to know about

None of the above are true in this case.

The error message is insufficient for the user or developer to diagnose the issue The user has no choice apart from to accept There is no clear user action which failed (the announcement appeared just fine)

Proposed:

Screenshots

IMG_0133

(please complete the following information):

Additional context Add any other context about the problem here.

alexbbrown commented 2 years ago

I searched for the string "Duplicate Record" in the Code Repo, but there was nothing: this must be coming from a lower level of the API.

alexbbrown commented 1 year ago

When my instance admin nova posts a notification, metatext displays this top right of timeline view with a red number.

when I tap this I almost always get the "duplicate record" modal error over the notification view

alexbbrown commented 1 year ago

I looked at what happens when you tap notifications, and saw that it calls the announcement API twice each time:

    // TableViewController.swift

    override func viewDidLoad() {
        …
        viewModel.request(maxId: nil, minId: nil, search: nil)
    }

    override func viewWillAppear(_ animated: Bool) {
        super.viewWillAppear(animated)

        refreshIfAble()
    }
…
    func refreshIfAble() {
        if viewModel.canRefresh {
            viewModel.request(maxId: nil, minId: nil, search: nil)
        }
    }

when the user taps the notification icon, the viewDidLoad is invoked and calls refresh, and so does viewWillAppear. Both of these cause an identical refresh from the server.

this issue might affect more than just the announcements view?

alexbbrown commented 1 year ago

note that I only get the "Duplicate Record" error when I start from a state with a new announcement in (which I can't reproduce readily-it depends upon my server admin). But it calls the API twice every time. I don't know why it fails in the one case but not the other.

alexbbrown commented 1 year ago

note that it is actually easy to cause the duplicate record error to occur in a different way: quickly double tap an emoji on the announcement screen. I found this because I have "boost" set to require double tap and my fingers thought emojis might work the same way. However, the reason this operation fails is different from the reason the announcement way fails.

alexbbrown commented 1 year ago

this issue might affect more than just the announcements view? yes, it does. Any time the TableView is created, TWO refresh calls are made. I don't know what overhead this makes or how often this is likely; but it's not good.

alexbbrown commented 1 year ago

I just got another notification (1) for my server and tapped it with http logging on. here are the responses it gets from the moment I tap the announcement icon which is showing a red 1, up until the activity stops, and the announcement is still on the screen

https://hachyderm.io/api/v1/trends Status Code: 200
https://hachyderm.io/api/v1/announcements Status Code: 200
https://hachyderm.io/api/v1/announcements Status Code: 200
https://hachyderm.io/api/v1/markers?timeline%5B%5D=notifications Status Code: 200
https://hachyderm.io/api/v1/announcements/71/dismiss Status Code: 200
https://hachyderm.io/api/v1/announcements/71/dismiss Status Code: 200
https://hachyderm.io/api/v1/announcements Status Code: 200
https://hachyderm.io/api/v1/announcements/71/dismiss Status Code: 200
https://hachyderm.io/api/v1/announcements Status Code: 200
https://hachyderm.io/api/v1/announcements Status Code: 200

while I wasn't hit with the duplicate message this time - perhaps because I let it run REAL slow while single stepping, its clear that too much activity is going on. In particular it seems likely that the multiple calls to dismiss might be interpreted as duplicate calls and cause a server error

For me I would expect this sequence instead:

https://hachyderm.io/api/v1/trends Status Code: 200
https://hachyderm.io/api/v1/announcements Status Code: 200
https://hachyderm.io/api/v1/markers?timeline%5B%5D=notifications Status Code: 200
https://hachyderm.io/api/v1/announcements/71/dismiss Status Code: 200
https://hachyderm.io/api/v1/announcements Status Code: 200
alexbbrown commented 1 year ago

contrast this to the same sequence when there is NO count - just the megaphone icon

https://hachyderm.io/api/v1/announcements Status Code: 200 https://hachyderm.io/api/v1/announcements Status Code: 200

this is still minor bug, doubling the call (see elsewhere) but much reduced.