DPLT-1034 feat: allow to edit indexer when forking #127

roshaans · 2023-07-13T19:48:26Z

While forking an indexer, you'll be given the chance to change the schema + indexerLogic before publishing.

When forking, we also update the indexer_name and account_name of the mutations and queries the new forked indexer targets.

-- What we do in this PR
Prior to this PR, the concept of forking an indexer(your own vs someone elses) or
publishing an indexer(creating a new indexer vs publishing code to an existing indexer) were very confusing.

With this PR, we separate the responsibilities to make it more clear. With that change, there is no concept of having an indexerConfig while forking an indexer as the only information we ask the user in the modal is about the new indexerName they would like to set.

In the background, we use the indexerConfig of the indexer the user forked.

roshaans · 2023-07-14T16:54:46Z

frontend/src/components/Editor/Editor.js

@@ -170,8 +152,8 @@ const Editor = ({
  const handleReload = async () => {
    if (isCreateNewIndexer) {
      setShowResetCodeModel(false);
-      setIndexingCode(defaultCode);
-      setSchema(defaultSchema);
+      setIndexingCode((formatIndexingCode(indexerDetails.code)));


When reloading, if you have forked an indexer, then it will reload the forked indexer rather than the default code.

This is a pretty nice change!

roshaans · 2023-07-14T16:56:36Z

frontend/src/components/Editor/EditorButtons.jsx

@@ -72,7 +72,7 @@ const EditorButtons = ({
                <Breadcrumb.Item className="flex align-center " href="#">
                  {accountId}
                </Breadcrumb.Item>
-                  {!isCreateNewIndexer && (
+                  {indexerName && (


when you have a forked Indexer, it has a indexerName. We want to show the indexerName as part of the breadcrumb paths.

But when we are creating a new indexer, it is fine not to show any breadcrumbs.

What is a Breadcrumb in this case?

Those are breadcrumbs. Weird name haha

darunrs · 2023-07-14T17:41:24Z

frontend/src/components/Editor/Editor.js

@@ -91,28 +91,6 @@ const Editor = ({
    localStorage.setItem(DEBUG_LIST_STORAGE_KEY, heights);
  }, [heights]);

-  // useEffect(() => {


This commented code seems to be doing something useful, checking for too large block heights. Is this code implemented elsewhere or do we not need to do this check anymore? Also the code on line 24 and 43 may not be needed since they reference things that are only present in the code being removed. Unless they're used by another class somewhere else. Also, given that this code is in FrontEnd, would it be worth it to add unit tests which pass in valid and invalid block heights?

We used to have a limitation in the number of blocks we would allow to be historically indexed. As a result we showed error messages if the user's set the block height prior to what our limit at that time was. I believe it was an hour or 3,600 blocks.

We have removed this limit now, and this code is not going to be needed anymore.

@gabehamilton thought that the relative block heights that we provided were quite helpful, so we still provide some helpful information regarding setting block height in this modal.

darunrs · 2023-07-14T17:46:41Z

frontend/src/components/Modals/ForkIndexerModal.jsx

@@ -14,14 +14,9 @@ export const ForkIndexerModal = ({ registerFunction, forkIndexer }) => {
    setIndexerConfig,
    isCreateNewIndexer,
  } = useContext(IndexerDetailsContext);
-  const [indexerConfig, setIndexerConfigField] = useState({


It seems to me that most of this part of the cleanup was removing the indexerConfig object. What was the purpose of the object before and why is it not necessary anymore?

Prior to this PR, the concept of forking an indexer(your own vs someone elses) or
publishing an indexer(creating a new indexer vs publishing code to an existing indexer) were very confusing.

With this PR, we separate the responsibilities to make it more clear. With that change, there is no concept of having an indexerConfig while forking an indexer as the only information we ask the user in the modal is about the new indexerName they would like to set.

In the background, we use the indexerConfig of the indexer the user forked. This makes things much more clear.

darunrs

Just a couple clarification questions and nitpicks but it LGTM otherwise.

darunrs

LGTM

roshaans · 2023-07-14T18:06:26Z

LGTM

Thank you. Moving forward, it will probably be really helpful to have a dev environment be automatically be created to test the changes in a PR. I'll look into how to do that.

roshaans · 2023-07-14T18:07:38Z

frontend/src/components/Editor/Editor.js

-  //
-  //   if (height - blockHeight > BLOCKHEIGHT_LIMIT) {
-  //     setBlockHeightError(
-  //       `Warning: Please enter a valid start block height. At the moment we only support historical indexing of the last ${BLOCKHEIGHT_LIMIT} blocks or ${BLOCKHEIGHT_LIMIT / 3600


Code that is not needed anymore.

feat: allow to edit indexer when forking

75c35f9

roshaans force-pushed the feat/edit-before-fork branch from 06ea896 to 75c35f9 Compare July 14, 2023 16:51

roshaans commented Jul 14, 2023

View reviewed changes

chore: clean up

0a87c56

roshaans force-pushed the feat/edit-before-fork branch from 93d168b to 0a87c56 Compare July 14, 2023 16:59

roshaans marked this pull request as ready for review July 14, 2023 17:00

roshaans requested a review from a team as a code owner July 14, 2023 17:00

roshaans changed the title ~~feat: allow to edit indexer when forking~~ DPLT-1034 feat: allow to edit indexer when forking Jul 14, 2023

darunrs reviewed Jul 14, 2023

View reviewed changes

darunrs requested changes Jul 14, 2023

View reviewed changes

darunrs self-requested a review July 14, 2023 17:58

darunrs approved these changes Jul 14, 2023

View reviewed changes

roshaans commented Jul 14, 2023

View reviewed changes

roshaans requested a review from morgsmccauley July 14, 2023 18:22

roshaans merged commit e9b66cc into main Jul 14, 2023

roshaans deleted the feat/edit-before-fork branch July 14, 2023 20:14

morgsmccauley mentioned this pull request Jul 25, 2023

DPLT-1072 Prod Release #145

Merged

morgsmccauley mentioned this pull request Apr 22, 2024

test stable branch git fix up #687

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DPLT-1034 feat: allow to edit indexer when forking #127

DPLT-1034 feat: allow to edit indexer when forking #127

roshaans commented Jul 13, 2023 •

edited

Loading

roshaans Jul 14, 2023

darunrs Jul 14, 2023

roshaans Jul 14, 2023

darunrs Jul 14, 2023

roshaans Jul 14, 2023 •

edited

Loading

darunrs Jul 14, 2023 •

edited

Loading

roshaans Jul 14, 2023

darunrs Jul 14, 2023

roshaans Jul 14, 2023

darunrs left a comment

darunrs left a comment

roshaans commented Jul 14, 2023

roshaans Jul 14, 2023

DPLT-1034 feat: allow to edit indexer when forking #127

DPLT-1034 feat: allow to edit indexer when forking #127

Conversation

roshaans commented Jul 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roshaans Jul 14, 2023 • edited Loading

Choose a reason for hiding this comment

darunrs Jul 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

darunrs left a comment

Choose a reason for hiding this comment

darunrs left a comment

Choose a reason for hiding this comment

roshaans commented Jul 14, 2023

Choose a reason for hiding this comment

roshaans commented Jul 13, 2023 •

edited

Loading

roshaans Jul 14, 2023 •

edited

Loading

darunrs Jul 14, 2023 •

edited

Loading