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

Do not insert all oid prefixes on db creation level #333

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

cymed
Copy link
Contributor

@cymed cymed commented Jul 23, 2024

General

  • Fix a bug

Context

It is not a good practice to store the oid prefixes in the main datamodel. The sysadmin should add their own prefixes to their datbase instead

Describe your changes

Remove provided oid prefixes

Screenshots

Issue ticket number and link

Checklist before requesting a review

  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • CI Tests are green
  • The documentation is up to date with the proposed change.
  • My work is ready for review

Checklist before merge

  • A review has been performed
  • Comments are resolved
  • Documentation is ready

It is not a good practice to store the oid prefixes in the main datamodel. The sysadmin should add their own prefixes to their datbase instead
@cymed cymed requested a review from sjib July 23, 2024 07:50
@cymed cymed self-assigned this Jul 23, 2024
@ponceta
Copy link
Member

ponceta commented Aug 8, 2024

I feel we should ease as much the configuration of things at least for TEKSI users and why not preconfigure oid prefixes?
We discussed about having an oid prefix switch in the TMMT so having several sample oid prefixes is a nice thing at least for testing purposes.

@cymed
Copy link
Contributor Author

cymed commented Aug 8, 2024

I feel we should ease as much the configuration of things at least for TEKSI users and why not preconfigure oid prefixes? We discussed about having an oid prefix switch in the TMMT so having several sample oid prefixes is a nice thing at least for testing purposes.

It is ok to have some fake oids for testing purposes, but I do not see an advantage in centrally storing all prefixes

@ponceta
Copy link
Member

ponceta commented Sep 4, 2024

In TMMT, I would like to be able to set the oid prefix from a dropdown / search menu :

image

Being a TEKSI member would enable you to have your oid_prefix already in that list without having to do sql or pgAdmin stuff.

@cymed
Copy link
Contributor Author

cymed commented Sep 5, 2024

In TMMT, I would like to be able to set the oid prefix from a dropdown / search menu :

image

Being a TEKSI member would enable you to have your oid_prefix already in that list without having to do sql or pgAdmin stuff.

I think it is better to have a GUI allowing to set that value instead. You'll have to do it once and nobody else needs that information.

Copy link
Contributor

@sjib sjib left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for me to take this out here. With more than 40 members and xxx communities this would become too big and difficult to maintain.

@sjib sjib added this to the TEKSI Wastewater 2024.0 milestone Dec 19, 2024
@ponceta
Copy link
Member

ponceta commented Dec 20, 2024

If we want to have INTERLIS checks in the future, we have at least to have an invalid and a valid prefix in the sample data.

@cymed
Copy link
Contributor Author

cymed commented Dec 20, 2024

@ponceta we can use the prefix ch24eax1, I created it for the TEKSI2AG-xx gateway. However, we should only import it for tests, not as part of creating od

@cymed
Copy link
Contributor Author

cymed commented Dec 20, 2024

@ponceta are you ok with merging this PR? Your comment is separate from it

@ponceta ponceta merged commit dcd929b into teksi:main Dec 20, 2024
7 checks passed
@cymed cymed deleted the drop-oid-prefixes branch December 20, 2024 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants