avniproject / avni-webapp

Web application for management and data entry
https://avniproject.org
GNU Affero General Public License v3.0
12 stars 42 forks source link

When a report card with already existing name created show proper error message #1297

Open mahalakshme opened 2 months ago

mahalakshme commented 2 months ago

Need to fix the comments and merge the PR. The contributor is missing.

Issue:

Create a report card with same name as non-voided existing report card. The below issue happens. Image

AC:

Show error message stating 'Report card with same name already exists' in the place where 'Name cannot be empty' is shown in the image below (or) when handling all these issues at server side is easier, then we can just show a common toast error message at the bottom if that will simplify the work across if in any new entities we face this issue.

Screenshot 2024-08-06 at 1 18 15 PM
Bhushan-Thombre commented 2 months ago

Hey @mahalakshme. Could you please assign me the issue.

mahalakshme commented 2 months ago

@Bhushan-Thombre sure you can work on it

Bhushan-Thombre commented 2 months ago

@mahalakshme Kindly review the below PR. https://github.com/avniproject/avni-webapp/pull/1301

himeshr commented 1 month ago

@petmongrels Created this draft sql script for fixing the duplicate report_cards https://github.com/avniproject/data-fixes/blob/main/all/duplicate_report_cards.md

dinesh2096 commented 1 week ago

Click here to watch video

mahalakshme commented 1 week ago

@himeshr @petmongrels In the SQL migration as well as in UI check, should be able to create a new report card, if the report card with the same name was voided before. Will make the AC more explicit next time. But have mentioned in the issue.

himeshr commented 1 week ago

Based on analysis so far, does not seem possible to implement DB level constraint which adheres to all the considerations below:

Based on following stackoverflow comments: https://stackoverflow.com/questions/16236365/postgresql-conditionally-unique-constraint https://stackoverflow.com/questions/64946724/how-to-deal-with-case-sensitivity-in-postgresql

Putting on hold for now.. for working on higher priority task.

Might be better to just add "(Voided ~~)" suffix to name

himeshr commented 5 days ago

We already rename voided report cards to include suffix voided~id. And with the existing unique constraint and case-insensitive check in sever, we should not be able to create cards with same name for an org, through the app. Not adding case-insensitive unique constraint using unique index, as it could fail transactions with updates of multiple report-cards

himeshr commented 5 days ago

SQL command to find already existing report-cards with case-insensitive match within same org.

select o.name, lower(rc.name), count(*)
from report_card rc
join organisation o on rc.organisation_id = o.id
    where not rc.is_voided
group by 1,2
having count(*) > 1;

In prod all report-cards are in decomissioned UAT orgs.

dinesh2096 commented 3 days ago

Query :

QA Reference video :

Click here to watch the video

himeshr commented 1 day ago

Have fixed the case reported here..

Query :

  • [ ] if we already created dashboard and if we try to edit it then error is popping up (eg : One card created as dummy and if the want change the name as Dummy then the error is showing as Report card already exists ) if so users want to rename the card then they want to delete and recreate the card

QA Reference video :

Click here to watch the video