Edit blogs, and add blog icons and banners
#460
Sloučený
elegaanz
sloučil 14 commity z větve blog-edit
do větve master
před před 5 roky
Posuzovatelé
Požádat o posouzení
Žádní posuzovatelé
Štítky
Zrušit štítky
Related to the REST API
Code running on the server
Stuff related to Federation
Related to the front-end
Translations, and related code
More about project management or code than the project itself
The building, or installation process of Plume
Something isn't working
We need to talk
New feature or request
This is a new feature
Compatibility with different browsers, readers and OS
Related to an external package that Plume uses
UI/UX related issues and PRs
Good for newcomers
Extra attention is needed
Issues affecting only mobile UX
How elements're rendered out for the end user
Something else needs to be fixed first
This issue or pull request already exists
This PR is not complete yet
Issues concern a limited number of instances
This doesn't seem right
Need to be discussed by the community (on Loomio)
This PR is ready to be reviewed
Proposed ideas worth considering
This is issue has been created after a vote on Loomio
This will not be worked on
Použít štítky
A: API
Related to the REST API
A: Backend
Code running on the server
A: Federation
Stuff related to Federation
A: Front-End
Related to the front-end
A: I18N
Translations, and related code
A: Meta
More about project management or code than the project itself
A: Security
Build
The building, or installation process of Plume
C: Bug
Something isn't working
C: Discussion
We need to talk
C: Enhancement
New feature or request
C: Feature
This is a new feature
Compatibility
Compatibility with different browsers, readers and OS
Dependency
Related to an external package that Plume uses
Design
UI/UX related issues and PRs
Documentation
Good first issue
Good for newcomers
Help welcome
Extra attention is needed
Mobile
Issues affecting only mobile UX
Rendering
How elements're rendered out for the end user
S: Blocked
Something else needs to be fixed first
S: Duplicate
This issue or pull request already exists
S: Incomplete
This PR is not complete yet
S: Instance specific
Issues concern a limited number of instances
S: Invalid
This doesn't seem right
S: Needs Voting/Discussion
Need to be discussed by the community (on Loomio)
S: Ready for review
This PR is ready to be reviewed
Suggestion
Proposed ideas worth considering
S: Voted on Loomio
This is issue has been created after a vote on Loomio
S: Wontfix
This will not be worked on
Bez štítku
A: API
A: Backend
A: Federation
A: Front-End
A: I18N
A: Meta
A: Security
Build
C: Bug
C: Discussion
C: Enhancement
C: Feature
Compatibility
Dependency
Design
Documentation
Good first issue
Help welcome
Mobile
Rendering
S: Blocked
S: Duplicate
S: Incomplete
S: Instance specific
S: Invalid
S: Needs Voting/Discussion
S: Ready for review
Suggestion
S: Voted on Loomio
S: Wontfix
Milník
Nastavit milník
Smazat milník
Žádné položky
Bez milníku
Zpracovatelé
Přiřadit uživatele
Smazat zpracovatele
Bez zpracovatelů
2 účastníků
Oznámení
Termín dokončení
Termín dokončení není platný nebo je mimo rozsah. Použijte prosím formát „rrrr-mm-dd“.
Žádný termín dokončení.
Závislosti
Nejsou nastaveny žádné závislosti.
Reference: Plume/Plume#460
Odkázat v novém úkolu
Není zde žádný obsah.
Smazat větev „blog-edit“
Smazání větve je trvalé. NEMŮŽE být vráceno zpět. Pokračovat?
Ne
Ano
Also adds a parameter to
md_to_html
to only render inline elements (so that we don't have titles or images in blog descriptions). And moves the delete button for the blog on the edition page.I still have to update the SQLite migration once others PRs with migrations will be merged.
Also, there will be a problem when you edit a blog while not owning its banner or icon: when validating they will be reset to their default values… I don't see a good solution to this until we have a better way to handle uploads with Rocket (the same is probably happening for articles btw).
And the icon/banner are not federated yet, I don't know if I should add it to this PR or if it can come after?
Fixes #453
Fixes #454
Codecov Report
👀
or all three?
@ -33,0 +47,4 @@
<a class="author p-author" href="@uri!(user::details: name = &author.fqn)">@author.name()</a>}
</p>
@Html(blog.summary_html.clone())
</main>
isn't the Avatar also optional?
@ -33,0 +47,4 @@
<a class="author p-author" href="@uri!(user::details: name = &author.fqn)">@author.name()</a>}
</p>
@Html(blog.summary_html.clone())
</main>
icon_url returns either the blog avatar, or the default one (see https://github.com/Plume-org/Plume/pull/460/files#diff-a4a32b57517e9459ff561610609f8044R364 )
Yes, at the time I wrote the comment
summary_html
was not here yet. 😁@ -33,0 +47,4 @@
<a class="author p-author" href="@uri!(user::details: name = &author.fqn)">@author.name()</a>}
</p>
@Html(blog.summary_html.clone())
</main>
duh, thanks!
👀
I find it a bit strange that one function return a default String and the other an Option (also,
unwrap_or(xxx.to_string())
, @igalic would get mad if it reached masterI'm not sure comments should be allowed to contain pictures and titles
@ -173,0 +282,4 @@
blog.summary = form.summary.clone();
blog.summary_html = SafeString::new(&utils::md_to_html(&form.summary, "", true).0);
blog.icon_id = form.icon;
blog.banner_id = form.banner;
user can provide the id of a media they don't own, or of a non-picture media
if let Some(banner) = blog.banner_url(ctx0)
would make more sense here, so there is no unwrap_or_defaultnot if my pr gets merged first
It makes sense to me that they don't have the same return type: in one case we provide a fallback URL, in the other case we just want to not display any image.
And I don't know what would be the correct way to replace this
unwrap_or
… 😅unwrap_or_else
because you're calling a function, so let's not call it if it's not necessaryDoes this function actually do what it says it does?
Looks like I pushed too fast… 😅
There is no way to remove pictures once you set theme, you can just replace with something else
👍
@ -47,3 +51,4 @@
pub banner_id: Option<i32>,
}
#[derive(Default, Insertable)]
I don't understand what this does, but apparently it did the trick
@ -47,3 +51,4 @@
pub banner_id: Option<i32>,
}
#[derive(Default, Insertable)]
It is an option for the
#[derive(AsChangeset)]
, that tells diesel to considerOption
fields that areNone
to be considered asNULL
in SQL, instead of not being included in the query at all.@ -47,3 +51,4 @@
pub banner_id: Option<i32>,
}
#[derive(Default, Insertable)]
Ok, doesn't this potentially break other things that relied on it before in our code?
@ -47,3 +51,4 @@
pub banner_id: Option<i32>,
}
#[derive(Default, Insertable)]
not according to our extensive tests
@ -47,3 +51,4 @@
pub banner_id: Option<i32>,
}
#[derive(Default, Insertable)]
Test in production ™️
@ -47,3 +51,4 @@
pub banner_id: Option<i32>,
}
#[derive(Default, Insertable)]
It only affects the
AsChangeset
implementation, which is used whenUPDATE
ing blogs, so I think it is the only place we are doing that.Posuzovatelé
bdfad844d7
.Krok 1:
Z vašeho repositáře projektu se podívejte na novou větev a vyzkoušejte změny.Krok 2:
Slučte změny a aktualizujte je na Forgejo.