这是indexloc提供的服务,不要输入任何密码
Skip to content

feat: toggle publish unpublish documents #421

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

Conversation

muzanella11
Copy link
Contributor

@muzanella11 muzanella11 commented Jul 31, 2024

What's new:

  • Button toggle for publish/unpublish document

Closes #380

Screenshot

Screen.Recording.2024-07-31.at.11.19.39.mov

@muzanella11 muzanella11 added this to the Publishing milestone Jul 31, 2024
@muzanella11 muzanella11 self-assigned this Jul 31, 2024
@muzanella11 muzanella11 marked this pull request as ready for review July 31, 2024 04:21
Copy link

github-actions bot commented Jul 31, 2024

Visit the preview URL for this PR (updated for commit 0424873):

https://tanam-testing--pr421-issue-380-toggle-pub-ervow6qu.web.app

(expires Fri, 09 Aug 2024 04:10:00 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 5bbe8a1a68d9684d656bffba10008fe532204561

const {update} = useCrudTanamDocument();

async function onTogglePublishDocument() {
let data = document?.toJson();
Copy link
Member

Choose a reason for hiding this comment

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

Do not downcast nice classes to JSON for logic.

JSON is only for serialization

  1. Saving data to database
  2. Transporting data to/from API endpoints

Keep the object as it is. Only cast it to JSON inside of hooks/services at the very moment when you are saving it to the database.


return (
documentId && (
<>
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need this empty <>. Please try remove it to confirm.

data = {...data, status: "unpublished", publishedAt: null};
}

await update(documentId, {...data});
Copy link
Member

Choose a reason for hiding this comment

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

You can make this method take in a single parameter of TanamDocumentClient type. Do the serialization to JSON inside of the hook.

Try to always keep the full instance of the class representation for all handling inside of the application and as much as possible when passing between methods. This allows you to isolate the internal logic of "how do we update things" inside of the hook.

This is better isolation of responsibilities instead of spreading out serialization logic across components.

const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState<UserNotification | null>(null);

async function update(data: Partial<ITanamDocument<Timestamp>>): Promise<void> {
async function create(documentType?: string) {
Copy link
Member

Choose a reason for hiding this comment

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

Perfect. This is good.

const [error, setError] = useState<UserNotification | null>(null);

async function create() {
async function update(documentId: string, data: Partial<ITanamDocument<Timestamp>>): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

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

Change this to

  async function update(tanamDocument: TanamDocumentClient): Promise<void> {

@DennisAlund DennisAlund merged commit 1d1d934 into milestone/3-publishing Aug 2, 2024
3 checks passed
@DennisAlund DennisAlund deleted the issue/380-toggle-publish-unpublish-documents branch August 2, 2024 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants