Url share by tfloxolodeiro · Pull Request #147 · Program-AR/pilas-bloques-app

tfloxolodeiro · 2023-12-05T18:57:05Z

Resolves Program-AR/pilas-bloques-ember#1290
Backend Program-AR/pilas-bloques-backend#16

… into url-share

asanzo

Buenas!!!
Me quedé sin tiempo para esto por hoy, me tengo que poner a hacer otras cosas. Dejo el comment con los comentarios que fui viendo.

¡No olvidar pensar algunos tests! Para empezar, el test que menciono en el back, si es que la lógica de cambio de usuario está acá en el front.

src/components/creator/Editor/CreatorContext.tsx

src/components/creator/SharedChallengeView.tsx

asanzo · 2023-12-05T21:27:59Z

src/components/creator/Editor/ActionButtons/ShareChallenge/ShareButton.tsx

+    const APP_URL = 'https://pilasbloques.program.ar/online'
+    const DEV_URL = 'localhost:3000'


Esto está mal, debería no estar hardcodeado. Si quieren pasen esto a un issue.

Opción A

Jugar con el window.location.href

Opción B

Estar en un env.

Ya lo hacemos para la API (donde no podemos jugar con el location.href) :

https://github.com/Program-AR/pilas-bloques-react/blob/12addc9af5d4032fb23439518543c9d2d329a3c4/src/pbApi.ts#L39

Podría ser en el .env (y en el sample.env) un REACT_APP_URL=localhost:3000

Pero OJO, que hay que cambiar tmb en el workflow en el momento del build, como acá:

https://github.com/Program-AR/pilas-bloques-react/blob/12addc9af5d4032fb23439518543c9d2d329a3c4/.github/workflows/build.yml#L79C3-L79C3

No hace falta que un secret (de hecho, para la api tampoco!), puede ser directamente el string:

Ahí podría ser REACT_APP_URL: 'https://pilasbloques.program.ar/online'

De hecho, si me lo pongo a pensar como app que cualquiera puede levantar en cualquier URL, la opción A es la más razonable.

asanzo · 2023-12-05T21:50:17Z

src/components/creator/SharedChallengeView.tsx

+    return (<>
+        {challengeExists ? (
+            <>
+                <Header/>


El Header debería decir "nombreDeUsuario>título del desafío" con un breadcrumb

… into url-share

…-share

asanzo

Felicitaciones, gente, LA BU RA ZO @dlopezalvas @tfloxolodeiro 👏 👏 👏

Lo que menciono en mi review anterior del header lo podemos patear. Dejé un par de comments abajo pero está mergeable, felicitaciones. ALTO FEATURE LOCO

asanzo · 2023-12-06T16:56:07Z

.github/workflows/build.yml

      runs-on: ubuntu-latest
      env:
        REACT_APP_API_URL: ${{ secrets.API_URL }}
+        REACT_APP_PB_APP_URL: ${{ secrets.APP_URL }}


La de arriba y esta pueden ser directamente

REACT_APP_API_URL: "https://pilasbloques.program.ar/api" REACT_APP_PB_APP_URL: "https://pilasbloques.program.ar/online/#/"

(no sé bien cómo deben ser los caracteres finales, pero puede ser así, no son secretas esas url)

(Idem en los otros jobs)

src/components/creator/Editor/ActionButtons/ShareChallenge/ShareModalButtons.tsx

asanzo · 2023-12-06T17:21:48Z

src/components/creator/Editor/ActionButtons/ShareChallenge/ShareModalButtons.tsx

+        const challenge: SerializedChallenge = LocalStorage.getCreatorChallenge()!
+        const sharedChallenge = await PilasBloquesApi.shareChallenge(challenge)
+        return sharedChallenge.sharedId


Esta lógica está repetida en ambos ShareUrlButton y SaveButton. Debería estar abajo en ChallengeUpsertButton.
Y si me apurás, también quiero el Snackbar del SaveButton en el botón compartir, porque después de todo estás guardando, está bien que le usuarie sepa que estamos guardando en ese momento.

el parámetro challengeUpsert no debería ser necesario.

asanzo · 2023-12-06T17:26:17Z

src/components/creator/Editor/ActionButtons/ShareChallenge/ShareModalButtons.tsx

Estos componentes quedaron re prolijos, gente, felicitaciones ❤️ . Estoy contento con cómo quedó. Hice un comment abajo sobre un posible refactor y pequeño cambio de lógica.

asanzo · 2023-12-06T17:30:57Z

src/components/creator/Editor/CreatorViewMode.tsx

        {challengeExists ? (
            <>
-                <Header CenterComponent={<CreatorViewHeader challenge={challengeBeingEdited} />} SubHeader={<EditorSubHeader viewButton={<ReturnToEditionButton />} />} />
+                <Header CenterComponent={<CreatorViewHeader title={challengeBeingEdited.title} />} SubHeader={<EditorSubHeader viewButton={<ReturnToEditionButton />} />} />


No entendí por qué pasa esto ahora...

asanzo · 2023-12-06T17:31:50Z

src/components/creator/SharedChallengeView.tsx

+    const challenge = LocalStorage.getImportedChallenge()
+
+    return <>
+        <Header CenterComponent={<CreatorViewHeader title={challenge.titulo} />} />


Epa, por qué el titulo viene en español??

asanzo · 2023-12-06T17:38:10Z

src/test/shareByUrl.test.tsx

+        const shareChallenge = async () => {
+            renderComponent(<CreatorContextProvider><ShareModal/></CreatorContextProvider>)
+            const shareButton = await screen.findByTestId('upsertButton')
+            await act(async () => {shareButton.click()})
+        }


asanzo · 2023-12-06T17:41:51Z

src/test/shareByUrl.test.tsx

VAMO TODAVÍA CON LOS TESTS DE FRONT 👏👏👏👏👏👏👏

src/test/shareByUrl.test.tsx

…reModalButtons.tsx Co-authored-by: asanzo <[email protected]>

Co-authored-by: asanzo <[email protected]>

… into url-share

dlopezalvas and others added 27 commits November 27, 2023 14:22

api share

9f00715

primer approach

6660793

cosas

f51c2ff

Merge branch 'url-share' of github.com:Program-AR/pilas-bloques-react…

4c27595

… into url-share

Boton compartir desafio

03553bf

Removing imports

cc6d9fc

shareId

d08d81c

savebutton

0e7f709

shareId fix

9dc0063

intelligence driven development

326b8b7

arreglando cagadas de diana

c8d8200

using shorter id

2ffe770

Merge branch 'url-share' of github.com:Program-AR/pilas-bloques-react…

703d669

… into url-share

cambiando ruta get

06dc08d

ChallengeUpsertButton

81eb7bb

serverError

c93dfaf

intl fix

e9d9ec7

tooltip

2872621

copy to clippboard (ugly)

49593c3

merge url-share

14824d0

files refactor + minWidth modal

f00cc87

fixing input adorment

b9df6f0

translations

c71387a

saved correctly

0affc09

fixing url

3267552

Merge branch 'url-share' of github.com:Program-AR/pilas-bloques-react…

ad2ab27

… into url-share

changing user challenge resources

ddaeaf3

tfloxolodeiro requested a review from a team as a code owner December 5, 2023 18:57

tfloxolodeiro requested a review from danielferro69 December 5, 2023 18:57

tfloxolodeiro mentioned this pull request Dec 5, 2023

Url share Program-AR/pilas-bloques-backend#16

Merged

dlopezalvas added 2 commits December 5, 2023 16:29

adding .env + share view mode

194fa50

using pb app url .env

3f52ff5

asanzo reviewed Dec 5, 2023

View reviewed changes

tfloxolodeiro added 6 commits December 5, 2023 19:17

share tests

0ce1ed2

+ tests

cb37b93

Merge branch 'url-share' of github.com:Program-AR/pilas-bloques-react…

ec92fd8

… into url-share

Merge remote-tracking branch 'origin/fixing-share-component' into url…

a25b047

…-share

fix env test

4dad523

sharedId

a915f09

asanzo approved these changes Dec 6, 2023

View reviewed changes

tfloxolodeiro and others added 4 commits December 11, 2023 14:20

Update src/components/creator/Editor/ActionButtons/ShareChallenge/Sha…

651e3dc

…reModalButtons.tsx Co-authored-by: asanzo <[email protected]>

Update src/test/shareByUrl.test.tsx

d658df5

Co-authored-by: asanzo <[email protected]>

ChallengeUpsertButton refactor

10d1229

Merge branch 'url-share' of github.com:Program-AR/pilas-bloques-react…

56d65be

… into url-share

tfloxolodeiro merged commit 6df07ff into develop Dec 11, 2023

tfloxolodeiro deleted the url-share branch December 11, 2023 17:56

		const APP_URL = 'https://pilasbloques.program.ar/online'
		const DEV_URL = 'localhost:3000'

Conversation

tfloxolodeiro commented Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asanzo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Opción A

Opción B

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asanzo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tfloxolodeiro commented Dec 5, 2023 •

edited

Loading