Skip to content

Allow for anonymous pulse submission and viewing in the pulse report.#2797

Merged
mkimberlin merged 5 commits intodevelopfrom
feature-2786/anonymous-pulse-responses
Dec 23, 2024
Merged

Allow for anonymous pulse submission and viewing in the pulse report.#2797
mkimberlin merged 5 commits intodevelopfrom
feature-2786/anonymous-pulse-responses

Conversation

@ocielliottc
Copy link
Collaborator

No description provided.

@ocielliottc ocielliottc linked an issue Dec 20, 2024 that may be closed by this pull request
Cookies.set(cookieName, 'true', { expires: 1 });
} else {
// Refresh browser to show that pulses where already submitted today.
history.go(0);
Copy link
Member

Choose a reason for hiding this comment

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

I know this wasn't you, but this is odd to me. Why are we refreshing the whole page to trigger a state change, I wonder... I blame @mvolkmann. I think he forgot how to write React code. 🤣

Copy link
Member

Choose a reason for hiding this comment

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

Now that I think about this. Couldn't this whole if/else be turned into just lines 106-107?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. I think we can. Currently, the cookie is only used when anonymous. But, we could do it generically too. Also, the refresh was weird to me too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

history.go(0) is the tool for when you can't find another way to get it to work and you are getting tired. ;-)

Copy link
Member

Choose a reason for hiding this comment

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

🤣🤣🤣

<div style={{ padding: '.3rem' }}/>
<label>
<Checkbox
disableRipple
Copy link
Member

Choose a reason for hiding this comment

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

The ripple haunts my dreams now.

@@ -0,0 +1 @@
ALTER TABLE pulse_response DROP CONSTRAINT pulse_response_teamMemberId_fkey;
Copy link
Member

Choose a reason for hiding this comment

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

It seems odd to me that we would need to drop a foreign key reference to allow null. Is this in some way related to: https://stackoverflow.com/questions/23325838/postgresql-null-value-in-foreign-key-column ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mkimberlin Honestly, I just assumed that because it was referencing the member_profile, that it could not be null. I did not test without this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed that file, it was not necessary and took out the history.go(0) call too.

@mkimberlin mkimberlin merged commit 988c2c9 into develop Dec 23, 2024
@mkimberlin mkimberlin deleted the feature-2786/anonymous-pulse-responses branch December 23, 2024 14:29
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.

Allow for anonymous pulse responses

3 participants