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.

Sign up for GitHub

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

Jump to bottom

fix: set publish as display #44

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: set publish as display #44

wants to merge 3 commits into from

Conversation

niklasvatn
Copy link
Contributor

@niklasvatn niklasvatn commented Mar 31, 2023 •
edited

Tried to go back in the history to see when this field disappeared buc could not see anything. Do you think it could be related to the switch from prometheus, @Herkarl ? Anyway, no it is being set explicitly on a post to admin/edit.

image

Resolves #40

@niklasvatn niklasvatn force-pushed the fix/nv/set-publish-as-display branch from cd2b021 to 41eba82 Compare March 31, 2023 12:55
@niklasvatn
Copy link
Contributor Author

image
Not sure exactly why publish_as_display is cleared when I change to publish under my on name. There is probably some logic somewhere that implements this.

@Herkarl
Copy link
Member

Herkarl commented Mar 31, 2023

Yeah, I imagine it has something to do with the move, form what I remember it only really old news items that had it set.

Herkarl
Herkarl previously approved these changes Mar 31, 2023
View reviewed changes
Copy link
Member

@Herkarl Herkarl left a comment

Choose a reason for hiding this comment

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

Seems like it should work, can't test it properly since calypso seems to refuse to use my mock-servers.

I think this would use the editors mandates rather than the publisher, right? Is there a risk of accidentally changing publishAs for another persons news item? (relevant for d-rek, d-sys and kommunikatör)

Doesn't set publishAs when creating new right now, which should be fairly easy, the same logic could probably be used.

@niklasvatn niklasvatn requested a review from Herkarl April 7, 2023 07:47
@niklasvatn @foodelevator
fix: set publish as display
fb7e6f4 
Tried to go back in the history to see when this field disappeared but
could not see anything. Do you think it could be related to the switch
from prometheus, @Herkarl? Anyway, no it is being set explicitly on a
post to `admin/edit`.

Resolves #40
@niklasvatn @foodelevator
fix: don't write over publish_as
1b9639b 
This is a problem in the live version as well. This is a weird fix, but
best I could do now. I think we should disable the `publish_as` selector
when someone but the user updates  the post.
@foodelevator
fix: only allow editing publish_as if you have the current mandate
98a4c52 
I think it would make sense to be able to change publish_as in other cases too, but
that would require fetching their names from dfunkt separately, so this seems like a
least-effort good-enough solution

With the behaviour before this commit, you could not change the mandate of an existing
post, even if you just created it (which to me seems unneccessarily restrictive). But
it seemed like you could in the frontend.
@foodelevator foodelevator requested review from Herkarl and removed request for Herkarl April 12, 2024 09:27
@niklasvatn
Copy link
Contributor Author

Wut. Are things happening here

@foodelevator
Copy link
Member

Det börjar bli dags

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elmaxe elmaxe Awaiting requested review from elmaxe

@Herkarl Herkarl Awaiting requested review from Herkarl

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix "publish as" option
3 participants
@niklasvatn @Herkarl @foodelevator