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

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

Open
Tracked by #1636
mahalakshme opened this issue Jul 29, 2024 · 9 comments
Open
Tracked by #1636
Assignees
Labels
good first issue Good for newcomers

Comments

@mahalakshme
Copy link
Contributor

mahalakshme commented Jul 29, 2024

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
Copy link

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

@mahalakshme
Copy link
Contributor Author

@Bhushan-Thombre sure you can work on it

@Bhushan-Thombre
Copy link

@mahalakshme Kindly review the below PR.
#1301

@petmongrels petmongrels self-assigned this Sep 17, 2024
petmongrels added a commit that referenced this issue Sep 18, 2024
…lated to server error handling. text color fix in snack bar.
petmongrels added a commit to avniproject/avni-server that referenced this issue Sep 18, 2024
…l. set a voided name for card being deleted. handle unique constraint error. added unique constraint for name.
@himeshr
Copy link
Contributor

himeshr commented Sep 20, 2024

@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 dinesh2096 self-assigned this Oct 17, 2024
@dinesh2096
Copy link

  • Need to handle the case sensitive.
  • when we edit the report card to the already existing report the web app displays the error

Click here to watch video

@himeshr himeshr self-assigned this Oct 18, 2024
himeshr added a commit to avniproject/avni-server that referenced this issue Oct 18, 2024
@mahalakshme
Copy link
Contributor Author

@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 added a commit to avniproject/avni-server that referenced this issue Oct 18, 2024
…ded entities only, while asserting unique name for reportCard
@himeshr
Copy link
Contributor

himeshr commented Oct 18, 2024

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

  • Case-insensitive check on report_card name
  • Ignoring voided entries
  • While retaining ability to perform bulk upload of report cards

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 added a commit to avniproject/avni-server that referenced this issue Oct 21, 2024
@himeshr
Copy link
Contributor

himeshr commented Oct 21, 2024

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 added a commit to avniproject/avni-server that referenced this issue Oct 21, 2024
@himeshr
Copy link
Contributor

himeshr commented Oct 21, 2024

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.

himeshr added a commit to avniproject/avni-server that referenced this issue Oct 21, 2024
…edFalse invocation capable of returning all matches
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Status: Code Review Ready
Development

No branches or pull requests

5 participants