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

fix: Revert "fix the gitSync related issue" #36658

Merged
merged 1 commit into from
Oct 3, 2024
Merged

Conversation

rishabhrathod01
Copy link
Contributor

@rishabhrathod01 rishabhrathod01 commented Oct 2, 2024

This reverts commit 026ef4e.

Description

Tip

Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).

Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.

Fixes #Issue Number
or
Fixes Issue URL

Warning

If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.

Automation

/ok-to-test tags="@tag.Sanity"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11146110986
Commit: 71a3b50
Cypress dashboard.
Tags: @tag.Sanity
Spec:


Wed, 02 Oct 2024 15:46:34 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • New Features

    • Updated action structure for fetching published page resources, improving clarity and consistency.
  • Bug Fixes

    • Removed outdated action handlers to streamline fetching logic and state management.
  • Documentation

    • Added comments in the AppViewer component to clarify the purpose of dispatch calls related to fetching page resources.

Copy link
Contributor

coderabbitai bot commented Oct 2, 2024

Walkthrough

The changes in this pull request primarily involve modifications to the pageActions.tsx, ReduxActionConstants.tsx, PageSagas.tsx, AppViewer/index.tsx, and appViewReducer.tsx files. Key updates include renaming the fetchPublishedPageResources function to fetchPublishedPageResourcesAction and altering associated parameters to enhance clarity. The action types related to fetching published page resources have been consolidated, with the removal of a success action type and the addition of an error type. These changes aim to streamline the Redux action management and improve the structure of action payloads.

Changes

File Path Change Summary
app/client/src/actions/pageActions.tsx Updated FetchPublishedPageResourcesPayload interface and renamed fetchPublishedPageResources to fetchPublishedPageResourcesAction. Modified parameters for createPageAction, setupPageAction, and setupPublishedPage.
app/client/src/ce/constants/ReduxActionConstants.tsx Removed FETCH_PUBLISHED_PAGE_RESOURCES_SUCCESS from AppViewActionTypes and added FETCH_PUBLISHED_PAGE_RESOURCES_ERROR to AppViewActionErrorTypes.
app/client/src/ce/sagas/PageSagas.tsx Updated fetchPublishedPageResourcesSaga to destructure applicationId instead of basePageId and removed dispatch for FETCH_PUBLISHED_PAGE_RESOURCES_SUCCESS.
app/client/src/pages/AppViewer/index.tsx Changed import from fetchPublishedPageResources to fetchPublishedPageResourcesAction and updated dispatch parameters accordingly.
app/client/src/reducers/uiReducers/appViewReducer.tsx Removed handlers for FETCH_PUBLISHED_PAGE_RESOURCES_INIT and FETCH_PUBLISHED_PAGE_RESOURCES_SUCCESS.

Possibly related PRs

Suggested labels

Release, Needs Triaging, ok-to-test

Suggested reviewers

  • sagar-qa007
  • ApekshaBhosale
  • albinAppsmith

🎉 In the world of code, changes unfold,
Actions and payloads, a story retold.
Fetching resources, now clearer and bright,
With names that align, everything feels right!
Redux flows smoothly, like a well-tuned song,
In the realm of development, we all belong! 🎶


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 688324e and 71a3b50.

📒 Files selected for processing (5)
  • app/client/src/actions/pageActions.tsx (2 hunks)
  • app/client/src/ce/constants/ReduxActionConstants.tsx (0 hunks)
  • app/client/src/ce/sagas/PageSagas.tsx (2 hunks)
  • app/client/src/pages/AppViewer/index.tsx (2 hunks)
  • app/client/src/reducers/uiReducers/appViewReducer.tsx (0 hunks)
💤 Files with no reviewable changes (2)
  • app/client/src/ce/constants/ReduxActionConstants.tsx
  • app/client/src/reducers/uiReducers/appViewReducer.tsx
🔇 Additional comments (7)
app/client/src/pages/AppViewer/index.tsx (2)

165-168: Let's examine this useEffect hook, class!

Good job updating the dispatch call to use our new fetchPublishedPageResourcesAction. It's like you've updated your vocabulary to use more precise terms!

However, I noticed a change in how we're passing parameters. Let's break it down:

  1. Old way: We were likely passing a single object with properties.
  2. New way: We're passing basePageId and baseApplicationId directly.

This change might affect how the action handles these parameters. So, here's your homework:

  1. Check the fetchPublishedPageResourcesAction definition to ensure it expects two separate parameters now.
  2. If other parts of the code call this action, make sure they've been updated too.

Remember, when we change how we call a function, we need to make sure the function is ready to receive the call in the new way!

Let's verify the action definition:

#!/bin/bash
# Check the definition of fetchPublishedPageResourcesAction
rg "export const fetchPublishedPageResourcesAction = \(" app/client/src/actions/pageActions.ts

31-31: Class, pay attention to this import statement change!

I see you've updated the import statement from fetchPublishedPageResources to fetchPublishedPageResourcesAction. This is a good change as it makes the action nature of the function more explicit. However, we need to make sure this change is consistent throughout our codebase.

Let's do a quick exercise:

  1. Check if all other files using this function have been updated.
  2. Verify that the function name in the actions/pageActions file has been changed as well.

Remember, consistency is key in coding, just like in your homework assignments!

To ensure consistency, let's run a quick search:

✅ Verification successful

Let's ensure we've updated all references correctly. Please run the following scripts to locate fetchPublishedPageResourcesAction and confirm all old references have been addressed:


Let's adjust our search to properly target TypeScript files. Please run the following scripts to locate fetchPublishedPageResourcesAction and ensure all old references have been updated:


**All references to fetchPublishedPageResources have been successfully updated to fetchPublishedPageResourcesAction. The renaming is consistent across the codebase, and no outdated usages remain. Great work!

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining usage of the old function name
rg "fetchPublishedPageResources[^A]" --type typescript

# Verify the new function name exists in the actions file
rg "fetchPublishedPageResourcesAction" app/client/src/actions/pageActions.ts

Length of output: 419


Script:

#!/bin/bash
# Search for the new function name across the entire codebase
rg "fetchPublishedPageResourcesAction" --type typescript

# Search for any remaining usage of the old function name
rg "fetchPublishedPageResources[^A]" --type typescript

Length of output: 183


Script:

#!/bin/bash
# Search for the new function name in TypeScript files
rg "fetchPublishedPageResourcesAction" -g "*.ts" -g "*.tsx"

# Search for any remaining usage of the old function name in TypeScript files
rg "fetchPublishedPageResources[^A]" -g "*.ts" -g "*.tsx"

Length of output: 680

app/client/src/actions/pageActions.tsx (4)

76-76: Class, pay attention to this important change in our interface!

We've updated our FetchPublishedPageResourcesPayload interface. Instead of using basePageId, we're now using applicationId. This is like changing from asking for a specific page in a book to asking for the whole book itself!

Remember, this change means we're thinking bigger - looking at the whole application instead of just one page. This could affect how you use this interface in your homework assignments, so make sure to update your code accordingly!


300-307: Now, let's examine this function transformation, students!

Our fetchPublishedPageResources function has undergone a makeover. It's now called fetchPublishedPageResourcesAction - a more descriptive name that tells us exactly what it does. It's like renaming "getStuff" to "fetchLibraryBooksAction" - much clearer, right?

Also, notice how we've split the parameters. Instead of bundling everything in one object, we now have pageId and applicationId as separate parameters. This is like asking for a book by its title and the library it's in, rather than just giving a vague description.

This change makes our code more explicit and easier to understand. Remember, clear function names and parameters are like good labels on your school supplies - they help everyone know what's what!


301-307: Let's talk about keeping track of our work, class!

We've made some exciting additions to our createPageAction function. It's like upgrading our attendance sheet to include not just names, but also which class and school each student is from!

We've added orgId and instanceId to our function. These are like special stamps that tell us more about where and how a page was created. And look at that new line with AnalyticsUtil.logEvent! This is like writing in our teacher's log book, keeping track of every new page we create.

Why is this important? Well, it's like being able to see not just that a new student joined the class, but knowing which school they came from and which program they're in. This helps us understand our app better and make smarter decisions about how to improve it.

Remember, good data is like good notes - the more details we have, the better we can understand what's happening!


301-307: Class, let's observe how we're making our functions work together!

Our createNewPageFromEntities function has gotten an upgrade too! It's like we've updated all our textbooks to use the same format - consistency is key in learning and in coding!

We've added applicationId, orgId, and instanceId to this function, just like we did with createPageAction. It's like making sure all our class projects follow the same template - it makes everything easier to understand and compare.

And look, we're using AnalyticsUtil.logEvent here too! This is like making sure we record attendance in every class, not just some of them. It helps us keep track of all the new pages we're creating, no matter how we create them.

One more thing - see how we've added applicationId to the payload? This is like making sure we always write which subject a homework assignment is for, not just the student's name. It gives us more context about where this new page belongs.

Remember, when our functions all work the same way, it's easier for everyone to understand and use our code. It's like having a consistent routine in class - it helps everyone know what to expect!

app/client/src/ce/sagas/PageSagas.tsx (1)

Line range hint 398-423: Class, let's examine the changes in our fetchPublishedPageResourcesSaga function.

Now, pay attention to the board. We've made some important updates:

  1. We've renamed basePageId to applicationId in our function parameters. This is a good change, as it more accurately describes what we're working with.

  2. In our API call, we're now using pageId instead of defaultPageId. This is an improvement in clarity and consistency.

  3. We've removed the FETCH_PUBLISHED_PAGE_RESOURCES_SUCCESS dispatch. This simplifies our code and reduces unnecessary actions.

  4. We're now using ConsolidatedPageLoadApi instead of separate API calls. This is an excellent optimization, children! It reduces the number of network requests we make.

These changes make our code more efficient and easier to understand. Well done!


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the Bug Something isn't working label Oct 2, 2024
@rishabhrathod01 rishabhrathod01 marked this pull request as ready for review October 2, 2024 14:07
@rishabhrathod01 rishabhrathod01 added the ok-to-test Required label for CI label Oct 2, 2024
@hetunandu hetunandu merged commit b582098 into release Oct 3, 2024
49 checks passed
@hetunandu hetunandu deleted the revert/pageSaga-fixes branch October 3, 2024 01:02
rishabhrathod01 added a commit that referenced this pull request Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working ok-to-test Required label for CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants