Исправление бага: можно создать записи в external_site_users с одинаковым external_id

[eugemos]

Что сделано

Поведение эндпойнтов api/auth/external_user_registration/fund и api/auth/external_user_registration/volunteer изменено в соответствии с условиями задачи #615. Дополнительно введён запрет на создание в external_site_users двух неархивных записей с одинаковым id_hash, так как наличие таких записей приводит к непредсказуемому поведению при регистрации пользователя в боте командой \start id_hash. Логика поведения данных эндпойнтов теперь такая (обновлено):

• если в запросе указан external_id, которого нет в БД:
  • если в БД есть неархивный пользователь с таким же id_hash, как в запросе, то выдаётся ошибка 400 "Пользователь с таким id_hash уже существует.";
  • если в БД есть архивный пользователь с таким же id_hash, как в запросе, то выдаётся ошибка 400 "Указанный id_hash не может быть установлен.";
  • иначе создаётся новый пользователь;
• если в запросе указан external_id, который есть в БД у архивного пользователя, то выдаётся ошибка 400 "Пользователь удален. Обновление невозможно."
• если в запросе указан external_id, который есть в БД у неархивного пользователя с пустым id_hash:
  • если в БД есть неархивный пользователь с таким же id_hash, как в запросе, то выдаётся ошибка 400 "Пользователь с таким id_hash уже существует.";
  • если в БД есть архивный пользователь с таким же id_hash, как в запросе, то выдаётся ошибка 400 "Указанный id_hash не может быть установлен.";
  • иначе найденный пользователь обновляется данными из запроса;
• если в запросе указан external_id, который есть в БД у неархивного пользователя с непустым id_hash:
  • если id_hash найденного пользователя совпадает с id_hash из запроса, то найденный пользователь обновляется данными из запроса;
  • иначе выдаётся ошибка 400 "Изменение id_hash у существующего пользователя запрещено."

Как проверял

Локально. Для обращения к эндпойнтам использовал страницу документации API. Для просмотра и внесения изменений в БД использовал DBeaver. При этом, перезапускал бот после изменений БД вручную. Проверил, что логика работы эндпойнтов соответствует заявленной выше.

[gorskyolga]

При тестировании поняла, что пользователь с существующим user_id не сможет изменить id_hash на какой-то другой свободный. Потенциально может быть полезно изменить id_hash (без создания дублирующей записи, как сейчас в develop).

[eugemos]

При тестировании поняла, что пользователь с существующим user_id не сможет изменить id_hash на какой-то другой свободный. Потенциально может быть полезно изменить id_hash (без создания дублирующей записи, как сейчас в develop).

Не уверен: id_hash, ведь, напрямую выводится сайтом из user_id, поэтому изменение id_hash при неизменном user_id выглядит сомнительно.

[gorskyolga]

Не уверен: id_hash, ведь, напрямую выводится сайтом из user_id, поэтому изменение id_hash при неизменном user_id выглядит сомнительно.

Только если хэширующая функция не будет изменена. Ладно, вероятность того, что её поменяют, маленькая. Наверное можно не предусматривать, что id_hash могут меняться.

[Xalgina]

Сейчас при попытке изменить id_hash у существующего пользователя возвращается 400 Bad Request “Изменение id_hash у существующего пользователя запрещено.” - так и должно быть, судя по последним комментам?

[Xalgina]

Сейчас при попытке изменить id_hash у существующего пользователя возвращается 400 Bad Request “Изменение id_hash у существующего пользователя запрещено.” - так и должно быть, судя по последним комментам?

Да, пока базовое поведение такое.

[eugemos]

Забавная история с этим id_hash получается. Вообще-то этот PR решает задачу о запрете создания записей с одинаковым external_id, про одинаковые id_hash в её условии ничего не говорится. Запрет на создание двух неархивных записей с одинаковым id_hash я ввёл по своей инициативе, потому что это прямо реальные грабли. Наличие же в БД нескольких записей с одинаковыми id_hash, из которых только одна неархивная, никаких проблем не создаёт, хотя, конечно, так быть не должно, т.е. это свидетельствует о каких-то проблемах в работе внешнего сайта. В связи с этим есть вопросы, @gorskyolga:

1. Точно ли надо запрещать такое поведение?
2. Если запрещать, то какое должно быть сообщение об ошибке? То, которое предполагает @Xalgina, на мой взгляд, не годится. Неправильно про архивного пользователя говорить, что он "уже существует", потому что он удалён, хотя и мягко. Вот, например, в аналогичном случае с external_id мы так и пишем, что "Пользователь удален. Обновление невозможно."

[gorskyolga]

Мне кажется, нужно запрещать создание двух записей (архивной и неархивной) с одинаковым id_hash.

1. Уникальность id_hash (в том числе между архивными и неархивными записями) - это базовое поведение в develop сейчас.
2. Метод get_by_id_hash предполагает возвращение одной записи. Если разрешить дубликаты id_hash с архивной записью, то при is_archived = None записи будет две, а метод будет возвращать одну запись. Это его еще рефакторить придется.
3. У нас есть эндпоинт "api/messages/group", который рассылает сообщения по id_hash (метод send_message_to_user_by_id_hash). Это единственное место, где сейчас используется id_hash, кроме регистрации и декоратора, проверяющего авторизацию. В нем также предполагается, что может быть только одна запись с id_hash. Этот эндпоинт бы конечно переделать на отправку по user_id, но не факт, что успеем это реализовать, так что его тоже нужно принимать во внимание.

Сообщение об ошибке можно сделать: "Указанный id_hash не может быть установлен.".

[eugemos]

Обновил описание.

[Xalgina]

Протестировано, ок. Запрещается создание двух записей (архивной и неархивной) с одинаковым id_hash с сообщением об ошибке: "Указанный id_hash не может быть установлен."