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

MBS-9832: Add tenant field to request_log table, increase precision of tenant fields #43

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

PhMemmel
Copy link
Member

@PhMemmel PhMemmel commented Jan 8, 2025

Closes #42

@marcusgreen
Copy link

This looks exciting. Will it need a change to import_from_xml in the questiontype file? I have a unit test that checks import and export.

@PhMemmel
Copy link
Member Author

Hi @marcusgreen, I don't think so. This is just about the fact that our internal tables have too low precision for the tenant identifier. The tenant is being fetched from the user table fields "idnumber" or "department" (there's an admin setting which to use). Those fields in the user table have a precision of 255 chars, our current tenant fields however only had 10.

The other change is just about the local_ai_manager internal logging table. We now also store the tenant the request has been used in to make sure that when a user changes a tenant the request log still "knows" on which tenant the request has been done so the statistics for this tenant are correctly calculated.

@marcusgreen
Copy link

Thanks for the feedback. I will take a closer look, which is much more about me understanding the code than any doubt about it :). Also I really want to keep my code aligned with yours. I am getting some good feedback about the Question type from several different countries.

@PhMemmel
Copy link
Member Author

As we are also using the qtype we will at least keep you informed when we are introducing breaking changes or even provide a pull request. Great to hear there is some positive feedback from all over the world!

@marcusgreen
Copy link

I was so distracted this morning that I had not realised it was your repo and your code, I thought I was looking at my own repo and my own code :(. Which may explain why some of my comments make little sense.

@PhMemmel
Copy link
Member Author

PhMemmel commented Jan 15, 2025

Great to hear you could resolve the confusion :) Don't worry!

@PhMemmel PhMemmel force-pushed the MBS-9832-Fix_tenant_database_fields branch from 7489092 to 5667689 Compare January 22, 2025 07:06
@PhMemmel PhMemmel changed the base branch from main to MBS-9807-Release-1.0 January 22, 2025 07:07
@PhMemmel PhMemmel force-pushed the MBS-9832-Fix_tenant_database_fields branch 2 times, most recently from 5198829 to 54de4ec Compare January 22, 2025 07:09

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…f tenant fields
@PhMemmel PhMemmel force-pushed the MBS-9832-Fix_tenant_database_fields branch from 54de4ec to 2aac873 Compare January 22, 2025 07:22
@PhMemmel PhMemmel merged commit 6ab43ae into MBS-9807-Release-1.0 Jan 22, 2025
24 checks passed
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.

Error when trying to enable tenant
2 participants