I'm not so sure this is a good idea. We usually reserve ID: 0 to indicate that an entity does not exist. This change would break that assumption. I'll let @camdencheek confirm that.

Since auto-defined contexts are not dynamic anymore, could we hardcode the single global context here: https://github.com/sourcegraph/sourcegraph/pull/44875/files#diff-2e0375121e614c40c04d23b1b05fc1a53d8281d83f0b02babdfc699e32098a31L164

That way, we can eliminate the entire concept of auto-defined contexts, and we treat global as a single special case.

IMO returning the global context from the backend makes the most sense, instead of doing this in the front-end. This would ensure the global context is always included in all cases including pagination, sorting, etc. The easiest way to do this is by adding it to the database. This also ensures that if users set their default context to "global" explicitly, or star the global context, there isn't a violation of foreign keys; otherwise, the constraints probably can't be handled on the DB side from what I understand. What alternative would you propose here?

(I was initially thinking of using the auto-incrementing ID here, but elsewhere in the backend we were explicitly checking if the ID is 0 for auto-defined contexts, so I thought this would simplify it and be ok; I can go back to the autoincrementing implementation).

I forgot about starring and defaulting to the global context, which complicates things.

We could use an auto-incremented ID for auto-defined contexts since you're already changing the check from "ID == 0" to "Autodefined == true". We would also have to prevent side admins from being able to edit auto-defined contexts.

Additionally, the global context won't be listed at the top anymore in the search context menu, right?

We usually reserve ID: 0 to indicate that an entity does not exist.

I don't think that really applies to DB interactions. In general, if a field is optional, it should be represented as a nullable type.

I'm struggling to decide whether or not the global context is actually a context, or whether it's the absence of a context. I think this matters for the implementation here. In practice, context:global means "ignore me." If the "global context" is not a context, and instead represents "no context", then it would feel weird to put it in the database because it's application logic rather than data. If global context is a context, and we add a context to every search query, then it seems like it should go in the database.

It does feel a little weird to insert data in a migration since the existence of the global context is implied. Especially since we depend on the ID being zero in some places, I'd feel better about it being added to the list at query time rather than inserting it into the db. We could even just create it in the query with something like UNION SELECT 0, "global", "All repositories on Sourcegraph"....

I also think it would be weird for the global context to not be always towards the top of the list. Probably even above starred contexts. I wouldn't want the global context to be paged out because a user starred 10 contexts or something

OK how about this:

• global always appears at the top of the list regardless of order. global cannot be starred. Setting global as default is done by removing the default for the user; it will show as default if the user doesn't have another default.
• If an explicit default is set, it appears second in the list
• If any contexts are starred, they appear after these, sorted by the orderBy param
• Any remaining contexts appear after that, sorted by the orderBy param

WDYT @quinnkeast?

That sounds perfect 👍 Thanks for hashing it out!

I've updated this PR to remove this insertion in the migration, and instead use a dummy row with UNION ALL. I'll update the sorting in #44876

Sounds good to me!

Two things:

1. IMO, SELECT * should generally be avoided. It makes it difficult to match up the number and order of the resulting columns with the destination variables in go. And the few extra lines is worth the avoided headaches later.
2. By explicitly selecting the columns, you can skip namespace_name, which has the awkwardness of being ignored by the row scanner on line 484.

I think namespace_name would still be required for the ORDER BY clause. I had to add it because apparently you can't use COALESCE() or other functions in a UNION. (See new line 191)

You can order by a field that you don't select. You will probably have to refer to it as t.namespace_name though

Yes, it worked! Thanks!!

I'll make this change on the ordering PR (#44876) to avoid merge conflicts.