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

No view logging for oss #39957

Merged
merged 11 commits into from
Mar 12, 2024
Merged

No view logging for oss #39957

merged 11 commits into from
Mar 12, 2024

Conversation

escherize
Copy link
Contributor

Follow on fix for #35288

We mustn't record view logging in OSS, but we were anyway.

@escherize escherize requested a review from camsaul as a code owner March 11, 2024 18:22
@escherize escherize requested a review from a team March 11, 2024 18:22
@metabase-bot metabase-bot bot added the .Team/AdminWebapp Admin and Webapp team label Mar 11, 2024
Copy link

replay-io bot commented Mar 11, 2024

Status Complete ↗︎
Commit 80cfe3d
Results
⚠️ 3 Flaky
2357 Passed

test/metabase/events/view_log_test.clj Outdated Show resolved Hide resolved
@luizarakaki luizarakaki added this to the 0.49 milestone Mar 11, 2024
@escherize escherize added the backport Automatically create PR on current release branch on merge label Mar 11, 2024
@escherize escherize merged commit 7fedfd6 into master Mar 12, 2024
105 checks passed
@escherize escherize deleted the no-view-logging-for-oss branch March 12, 2024 20:48
escherize added a commit that referenced this pull request Mar 12, 2024
* move log-enabled? into premium-features

* only `record-views!` when `log-enabled?`

* test that view-log entries arent created in oss

* simplify test

* fix new test fails: we don't log views in OSS.

* fix 3 more view logging tests (they dont do anything in oss)

* fix dashboard-read-ee-test

- check for `log-enabled?` not `ee-available?`

* fix the rest of the tests

- Only check for view logs when (premium-features/log-enabled?) is falsey

* adjusting e2e test

---------

Co-authored-by: Nick Fitzpatrick <nickfitz.582@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge .Team/AdminWebapp Admin and Webapp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants