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

create additional unique constraints #59

Closed
patrickakk opened this issue Jan 31, 2023 · 20 comments
Closed

create additional unique constraints #59

patrickakk opened this issue Jan 31, 2023 · 20 comments
Assignees
Labels
admin Admin - Part of application done Development and review process finished. Ready for release - Status of issue featurerequest New feature request - Kind of issue
Milestone

Comments

@patrickakk
Copy link
Contributor

currently it's possible to enter duplicate data in the admin area. the following items should be unique: cities, countries, courses: on institution_id & course name, faq_questions.name, invite_translations.language_id, languages

create these rules in the database and application.

fix conflicts when they occur.

@patrickakk patrickakk self-assigned this Jan 31, 2023
@patrickakk patrickakk added the bug Something isn't working - Kind of issue label Feb 4, 2023
@patrickakk
Copy link
Contributor Author

A unique for the table courses on institution_id & course name is required for the search bar (#62) , since the lookup depends on this being unique.

@patrickakk patrickakk added the indev Currently in development – Status of issue label Apr 25, 2023
@patrickakk patrickakk added this to the mai23 milestone Apr 25, 2023
@patrickakk
Copy link
Contributor Author

patrickakk commented Apr 25, 2023

-removed content of this comment-

reason: moved all conflicts to a list

@patrickakk patrickakk added indev Currently in development – Status of issue and removed indev Currently in development – Status of issue labels May 6, 2023
@patrickakk
Copy link
Contributor Author

unique for the table courses on institution_id & course name implemented in local test database. 58 conflicts found. spreadsheet with proposed solutions is available.

@patrickakk patrickakk added blocked Blocked by other action – Status of issue and removed indev Currently in development – Status of issue labels May 8, 2023
@patrickakk
Copy link
Contributor Author

waiting for decisions in next meeting

@patrickakk
Copy link
Contributor Author

The spreadsheet with suggested solutions to fix the conflicts can be found here:
https://docs.google.com/spreadsheets/d/1V6rUDUKE34EhwGgMYg0dylOUwJB0nlLP6wlSnKgeeZY/edit?usp=sharing

@patrickakk patrickakk removed the blocked Blocked by other action – Status of issue label May 11, 2023
@patrickakk patrickakk removed their assignment May 11, 2023
@patrickakk
Copy link
Contributor Author

Waiting for reaction on proposed solutions in spreadsheet.

@patrickakk patrickakk added the inreview Request from dev to test and give feedback on the implementation of the issue - Status of issue label May 11, 2023
@patrickakk
Copy link
Contributor Author

@PixlTracer @IvdL22

Update:
Based on the fact that the Education Type is also shown in the list view, I've updated the unique rule, so that now the combination of Course Name + Institution + Education Type needs to be unique. This avoids adding a Education Type in the name and duplicating data.

In the spreadsheet linked above, I've marked the lines we can ignore in grey/black. As you can see in column G, only 21 courses are left, that's were we need to take a decision about.

Can you tell if the proposed changes in column H can be made?

@PixlTracer
Copy link

PixlTracer commented Jun 13, 2023

Hi Patrick
I finally managed to go through the list; I have added my comments in column I.

while checking the courses it came to my mind, that this might happen again in the future:
some courses taught in different cities all over Europe might have the same name (e.g. introduction to digital humanities), some courses taught at a certain university might have the same name too, e.g. Digital Humanities (in both BA's & MA's);

Hence, the following idea came to my mind:
when using the search bar, a user is entering certain keywords - why not output a list (similar to when applying filters) of all the courses that match the keyword? so we don't need to check the course-names for their uniqueness...

very much looking forward to the deployment of the new feature!

@patrickakk
Copy link
Contributor Author

patrickakk commented Jun 15, 2023

@PixlTracer Thanks for taking a look at the list. I'd like to divide you comment in thee parts.
(cc @IvdL22 )

while checking the courses it came to my mind, that this might happen again in the future:
some courses taught in different cities all over Europe might have the same name (e.g. introduction to digital humanities), some courses taught at a certain university might have the same name too, e.g. Digital Humanities (in both BA's & MA's);

so we don't need to check the course-names for their uniqueness

In the future we don't have to do that. That is what the implementation of the combination of Course Name + Institution + Education Type being unique means. As soon as this is in production, the application will do that for us.
What we are doing right now, is cleaning up after 9 years running without this validation rule having implemented.

Hence, the following idea came to my mind:
when using the search bar, a user is entering certain keywords - why not output a list (similar to when applying filters) of all the courses that match the keyword?

Can we continue the talk about the search bar in the corresponding issue? Here:
#62 (comment)

I finally managed to go through the list; I have added my comments in column I.

Thanks for your suggestions, I have a few remarks.

  • In the last meeting we agreed to add “(old)” to older courses which are duplicate. Now you suggested to add "(deprecated)" instead. Do we all agree on this?

  • In other case you suggested to include the education type into the course name. This would be duplicating data, since the education type is already stored in a separate field and shown in a separate column in the main page?
    So I would suggest not add the education types in the course name.

  • In a few cases you rewrote the course name? My idea was that we as administrators usually don't change data and leave it the way it's entered by the users. The only exception would be adding old/deprecated to implement the additional validation rules. We could leave the course name unchanged?

Link to the spreadsheet (again): https://docs.google.com/spreadsheets/d/1V6rUDUKE34EhwGgMYg0dylOUwJB0nlLP6wlSnKgeeZY/edit?usp=sharing

Based on your suggestions, I've made a new proposal in column J. Can it be changed like that?

@patrickakk patrickakk added the admin Admin - Part of application label Jun 15, 2023
@patrickakk
Copy link
Contributor Author

As discussed in the meeting today, the proposal in column J is agreed and the changes can be made.

@patrickakk patrickakk assigned patrickakk and unassigned IvdL22 and PixlTracer Jun 21, 2023
@patrickakk patrickakk added indev Currently in development – Status of issue and removed inreview Request from dev to test and give feedback on the implementation of the issue - Status of issue labels Jun 21, 2023
@patrickakk patrickakk added this to the November 2023 milestone Sep 6, 2023
@patrickakk
Copy link
Contributor Author

No time available for this now, moved to November milestone.

It's still important because it contributes to the quality of the data.

@patrickakk patrickakk removed this from the November 2023 milestone Nov 15, 2023
@patrickakk patrickakk removed their assignment Nov 15, 2023
@patrickakk patrickakk added this to the 2024 milestone Dec 5, 2023
@patrickakk patrickakk modified the milestones: Items to keep, 2024-12 Nov 8, 2024
@patrickakk patrickakk self-assigned this Nov 8, 2024
@patrickakk patrickakk added indev Currently in development – Status of issue and removed todo Ready to start development - Status of issue labels Nov 8, 2024
@patrickakk
Copy link
Contributor Author

patrickakk commented Nov 9, 2024

  • invite_translations - language_id
  • faq_questions - question
  • countries
  • cities
  • languages

@patrickakk patrickakk assigned IvdL22 and PixlTracer and unassigned patrickakk Dec 8, 2024
@patrickakk patrickakk added inreview Request from dev to test and give feedback on the implementation of the issue - Status of issue and removed indev Currently in development – Status of issue labels Dec 8, 2024
patrickakk added a commit that referenced this issue Dec 8, 2024
@patrickakk
Copy link
Contributor Author

patrickakk commented Dec 8, 2024

@PixlTracer @IvdL22

Can you review this?

Validation has been implemented for

The remaining tasks have been split into new issues, as you can see above, so that we can continue and release the work which is done (also other issues), when the review process is finished.

@PixlTracer PixlTracer removed the inreview Request from dev to test and give feedback on the implementation of the issue - Status of issue label Dec 10, 2024
@PixlTracer
Copy link

hi @patrickakk
I've checked this, looks fine!
thank you for the effort! I removed the "in review" label (and did not add 'done' label since there are a couple of more items on the list above, please assign 'done' label in case i misread)

@PixlTracer PixlTracer removed their assignment Dec 10, 2024
@patrickakk patrickakk added the done Development and review process finished. Ready for release - Status of issue label Dec 11, 2024
@patrickakk
Copy link
Contributor Author

@PixlTracer
cc @IvdL22

Yes, this might be a bit confusing. There are indeed three check boxes unchecked above. Those three tasks are split off into three new issues (120, 121, 122) as also listed above. The checkboxes remain unchecked and this issue can be closed. The actual tasks are in the new issues.

Maybe this would have been a better idea in the first place, to split into smaller issues, since this single issue contained a lot of work and has been open for a long time.

And because somebody reported a bug about the sorting of the lists, there was less time available for this issue ;)

@patrickakk
Copy link
Contributor Author

Implemented in 2024-12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admin Admin - Part of application done Development and review process finished. Ready for release - Status of issue featurerequest New feature request - Kind of issue
Projects
None yet
Development

No branches or pull requests

4 participants