fix: ensure private meta-analyses do not appear in the meta-analyses …#1349
fix: ensure private meta-analyses do not appear in the meta-analyses …#1349nicoalee wants to merge 3 commits into
Conversation
jdkent
left a comment
jdkent
left a comment
There was a problem hiding this comment.
Good work! You working on this gave me insight into the assumptions I hold when writing code for the backend.
| type = Column(Text) | ||
| estimator = Column(JSON) | ||
| filter = Column(Text) | ||
| filter = Column(JSON) |
There was a problem hiding this comment.
Changing the column type in models typically means there needs to be a migration script in the database to update the postgres table column.
| # Support optional auth on public endpoints by decoding the bearer token | ||
| # if provided. This keeps public routes functional while still honoring | ||
| # user-specific visibility when an Authorization header is present. | ||
| auth_header = request.headers.get("Authorization") | ||
| if not auth_header: | ||
| return None | ||
| try: | ||
| token = get_token_auth_header() | ||
| except AuthError: | ||
| return None | ||
| bearerinfo_func = current_app.config.get("BEARERINFO_FUNC") | ||
| if isinstance(bearerinfo_func, str): | ||
| module_name, func_name = bearerinfo_func.rsplit(".", 1) | ||
| bearerinfo_func = getattr(importlib.import_module(module_name), func_name) | ||
| if callable(bearerinfo_func): | ||
| try: | ||
| token_info = bearerinfo_func(token) | ||
| except Exception: # noqa: BLE001 | ||
| return None | ||
| if token_info and token_info.get("sub"): | ||
| return User.query.filter_by(external_id=token_info["sub"]).first() | ||
| return None |
There was a problem hiding this comment.
I prefer to handle this through the openAPI specification/connexion.
| ``` | ||
|
|
||
| If you're using the legacy Postgres container, replace `compose_pgsql17` with `compose_pgsql` in the commands above. | ||
| If you're using the legacy Postgres container, replace `compose_pgsql17` with `compose-pgsql` in the commands above. |
There was a problem hiding this comment.
Is there a service container named compose-pgsql? (I started replacing underscores with hyphens so the container services would create valid URIs for looking at database status with pghero/grafana; the changes in the names of the services is after I switched from postgres12 to postgres17).
There was a problem hiding this comment.
ah apologies. when i spin everything up in my local, there are still some parts of the code that expect a hyphen in order to start up the services so i wasnt sure which one was correct. feel free to disregard this change
There was a problem hiding this comment.
ah, are those inconsistencies in the codebase? I can update those. It would be beneficial if I added some developer notes for getting everything setup/quirks of the system.
There was a problem hiding this comment.
i dont remember exactly what i had to do. next time i spin up compose from scratch, ill make a note of what i needed to change and make a PR
| @hybrid_property | ||
| def public(self): | ||
| """Meta-analysis inherits public status from parent project""" | ||
| if self.project: | ||
| return self.project.public | ||
| return True # Default to public if no project | ||
|
|
||
| @public.expression | ||
| def public(cls): | ||
| """SQL expression for querying public meta-analyses""" | ||
| return func.coalesce( | ||
| select(Project.public) | ||
| .where(Project.id == cls.project_id) | ||
| .scalar_subquery(), | ||
| True, | ||
| ) | ||
|
|
||
| @hybrid_property | ||
| def draft(self): | ||
| """Meta-analysis inherits draft status from parent project""" | ||
| if self.project: | ||
| return self.project.draft | ||
| return False | ||
|
|
||
| @draft.expression | ||
| def draft(cls): | ||
| """SQL expression for querying draft meta-analyses""" |
There was a problem hiding this comment.
If this is being used as a filtering step (filter out meta-analyses in private projects, then using these correlated queries will likely be slower than using a join with the project table). I'll test an example.
|
Perhaps the solution I want is to have both projects and meta-analyses to have their own public/private mode, then when a project is made public, the update will also touch meta-analyses. With separate "public" properties, a project could be public with private meta-analyses, and vice versa. I could see use cases for this (want to share how the data were curated, but not your analyses; or want to share your analyses, but not how your data was curated). These are not strong use cases, but it is easier (less code) just to add a "public" property to meta-analyses and sync when projects are updated. |
|
I made the backend change in #1403.
|
2 participants
…page