T11092 has a lot of discussion, but D16031 will almost certainly prevent this from doing damage. It does not fix the underlying Quicksand issue, but you should get an immediate error instead of confusing/spooky behavior/apparent data loss.
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
All Stories
Jun 3 2016
Assuming D16031 is a common element, you can identify all comments with inconsistent state like this:
It's not permanent, but this sequence would cause the problem:
Can you define 'never'? As in, is this the kind of thing that if you accidentally open conpherence once you're forever banished to the land of AJAX bug hell? Or do you mean, at no time during his review process? This has been going on intermittently since February so I am confident all users haven't done this every time.
Is he 100% sure he has never touched the \ key on his keyboard to bring up the chat sidebar?
I spoke with rkeelan/Rob. Here is what he had to say..
I was able to reproduce some flavor of this for inline comments with Quicksand enabled (i.e., the persistent Conpherence chat sidebar open). That's probably T8852, but I expect it to be specific to Quicksand / client-side JS state and impossible to hit without pressing the \ key to open the chat column. I should be able to fix this, but I can't repro it without opening the column.
Another user description of this issue:
There's no possible way that PDF is about inline comments, I think? How do inline comments appear on different reviews which presumably have different files? Or did they happen to affect the same files?
Particularly useful, then, would be to find these inlines in the database and figure out what happened to them.
I have 14.10 on a VM to play with in the meantime.
It might be good to round up any system "commonalities" from users who have experienced this. Perhaps a slowvote poll?
:| yes. these are inlines.
I emailed evan a "print page" pdf of the ticket off of our install.
That's like... 2.00 revisions ago, by maths.
14.04
which ubuntu can we fire up? 16.04?
Haha, okay. I may be headed down the wrong path -- T5031 is almost certainly not related here, because the draft system and EditEngine don't interact directly with inline comments. This is almost certainly also not Quicksand-related either if the inlines appear correctly originally (Quicksand could potentially mangle request in-flight).
INLINE COMMENTS BRAD
Oh, are "comments" in this description inline comments, versus blocks of comment text?
I think that this has only ever happened on Chrome on Ubuntu, and to boot, we've seen client side Ubuntu bugs pop up in the past. (Brad do you remember where?)
Thanks for the quick research/reply.
We can blindly move Differential to EditEngine, which may fix this indirectly, but this is a large amount of work.
Although I'm not sure this has the same root cause as T8852, I can't reproduce that either.
Here's how I attempted to reproduce this:
When the Conpherence column is open, it activates a completely separate browsing mode (Quicksand) where all navigations occur over Ajax. This is expected to have weird bugs associated with it, but this mode is not active if the column is not open. If the users this is affecting have never opened the chat column, I suspect this is not the same as T8852.
In particular, T182 discusses a desired "chain of custody" workflow where the merge/promote flow happens on the server.
We can only move forward with complete reproduction steps that allow us to reproduce this problem in a local environment.
Unfortunately, it would certainly be impossible to reproduce our AD as it is fairly large, but the settings are very straight forward. I found the following in my apache error log, perhaps it's possible to determine what happened based off of this.
None of those screenshots demonstrate inconsistencies on their own.
Okay. I can't reproduce that. Here's what I did.
The author of the revision intended to mark the comment Done, but this didn't happen when he submitted the form. For me that seems to be unexpected behaviour.
The "Inline Comments" section shows new inline comments. It does not show inlines marked "Done".
Thanks for the feedback. I'll sit tight. You know how to reach me if there are any problems or requests related to this. :-)
@epriestley does reviews from contributors, but I think we likely have hundreds outstanding right now. See Planning.
Not wanting to nag, but it's been a couple of months since I submitted a diff that fixes this. Would it be possible to include it? Have I done something wrong? Perhaps @chad would know?
Jun 2 2016
Here's at least one specific reproduction case which still isn't working well:
Confirm bug that is, no idea about SQL query.
I can confirm.
Will the following work to fix this?
To summarize the above: if there are two renames, the first one loses information if we don't include the old and new name. Current behavior:
I mean in the web notifications.
I'm just going to merge this into T11076 since this will happen with the rest of the deployment work there. I expect this to be straightforward.
From the downstream, it seems like the problem may be one specific user wanting to add mirrors to existing repositories:
If you tell me what username you would like and what your email is, I can create an account for you.
It's also vaguely possible that we'll add "suggest an edit" (ala T4104) in a generic way, then untrusted users could suggest edits and trusted users could approve them. I don't currently plan to build this as a general-purpose workflow, though.
Another thing to consider is that your worries may be entirely solved at a social level. Giving people access but reminding them to only modify 'safe' fields. If your users are somewhat trustworthy this is entirely reasonable.
I don't plan to pursue field-level permissions in any application. They were somewhat recently removed from Maniphest in T10003. That task discusses some of the complications that field-level permissions create in the general case.