Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model HoDan nên chỉnh gấp #224

Open
DangHoangGeo opened this issue Nov 5, 2020 · 3 comments
Open

Model HoDan nên chỉnh gấp #224

DangHoangGeo opened this issue Nov 5, 2020 · 3 comments
Labels
enhancement improvement for existing feature need more info

Comments

@DangHoangGeo
Copy link
Collaborator

DangHoangGeo commented Nov 5, 2020

Hiện tại mình đang đảm nhiệm issue #208, và nhận thấy có một số điểm nên sửa luôn. Do đó mình tách riêng thành issue mới.

1. Hiện tại các trường như status, volunteer, cuuho của model HoDan đang chứa thuộc tính on_delete=models.CASCADE.

Vấn đề: khi muốn xoá một trường khoá ngoại của các model status, hoặc volunteer thì sẽ bị yêu cầu xoá luôn tất các các hộ dân đang tham chiếu đến các trường này.

Đề xuất: nên chuyển thành on_delete=models.SET_NULL

2. Hiện tại tất cả các trường đều đang để blank=True, và có sẵn default. dẫn đến tình trạng có nhiều thông tin rác trên danh sách hộ dân.
Đề nghị : bỏ blank=True và default='' của một số trường sau: name('Tiêu đề'), location('Địa chỉ'), status và cả note('Ghi chú')
Theo mình những thông tin này cần bắt buộc và đi liền với nhau để bên cứu hộ có đủ thông tin cần thiết.
Phone thì cũng nên xem xét.
Mong mọi người đóng góp thêm ý kiến.

@DangHoangGeo DangHoangGeo changed the title Model HoDan Model HoDan nên chỉnh gấp Nov 5, 2020
@DangHoangGeo DangHoangGeo added high priority enhancement improvement for existing feature and removed high priority labels Nov 5, 2020
@dangquangdon
Copy link
Contributor

dangquangdon commented Nov 5, 2020

Mình cũng có để ý đến vấn đề này, và đồng tình với @DangHoangGeo về việc phải sửa cái contraint này. Tuy nhiên, mình có ý kiến là nên extend PR ra cho các model khác để sửa 1 thể luôn.

Ngoài contraint on_delete ra, cũng có vấn đề về việc đặt ForeignKey ở 1 số model như TinhNguyenVien, CuuHo, HoDan đều có đặt ForeignKey tới tất cả Tinh, Huyen, XaThon. Như này khả năng sẽ gây corrupt data. Bởi lẽ Tỉnh - Huyện - Xã - Thôn đã có foreign key với nhau thì phải được link đúng. Ví dụ như user có thể nhập Tỉnh A, thì chỉ được chọn Huyện trực thuộc Tỉnh A, ko thể chọn Huyện mà thuộc vào tỉnh B đc. Với việc đặt foreignkey như hiện h, thì hoàn toàn có thể xảy ra trường hợp nhầm lẫn.

Vấn đề này mình không chắc là có thể giải quyết được ở model level mà chắc phải giải quyết ở form. Và phải hy vọng + tin tưởng rằng nếu ai đó nhập/sửa data ở shell sẽ không screw up 🤣

Edited: Nevermind, đọc kỹ lại code thì mình thấy các bạn đã implement cái này vào form rồi. 👍

@kc97ble
Copy link
Collaborator

kc97ble commented Nov 6, 2020

Mình nghĩ nên thực hiện việc thay đổi này sau khi bão số 10 kết thúc và trước khi bão số 11 tới.

Có một số vấn đề cần được làm rõ trước khi chuyển nhãn từ need more info sang high priority:

  1. Việc sửa đổi này có tương thích với dữ liệu cũ không?

  2. Nếu không, xử lý các mục không tương thích như thế nào?

  3. Có cần riêng một ngày để migrate data hay không?

  4. Việc sửa đối này có ảnh hướng tới các tính năng khác, ví dụ dashboard, API tới các bên thứ ba hay không?

@dangquangdon
Copy link
Contributor

  • Việc sửa lại on_delete thì không ảnh hưởng gì tới database hiện tại, chạy file migration là ổn.

  • Việc sửa blank=Truedefault= "" thì ko nên, nó ko improve đc gì lắm, lại có thể phiền phức trong việc phải migrate data, cũng như việc phải sửa lại những form/ form data hiện h đang sử dụng. Thêm nữa, nếu là người dùng, có những khi vội mà phải điền những field dài ko cần thiết thì ko hay lắm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improvement for existing feature need more info
Projects
None yet
Development

No branches or pull requests

3 participants