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

[Modern] bug when updating json scalar field #2049

Closed
jardakotesovec opened this issue Aug 21, 2017 · 10 comments
Closed

[Modern] bug when updating json scalar field #2049

jardakotesovec opened this issue Aug 21, 2017 · 10 comments

Comments

@jardakotesovec
Copy link
Contributor

I am using JSON Scalar field.

And getting error:

recycleNodesInto.js:48 Uncaught (in promise) TypeError: Cannot assign to read only property 'finishingGroupIds' of object '#<Object>'
    at recycleNodesInto.js:48

when relay store receives update from the mutation with new json scalar value, which has internally additional item in the array. When I reload the page - store interpret the json value just fine, so its just issue when replacing the old json value with new one.

This whole recycleNodesInto function already seems to be doing some magic for flow, so not sure where is actually the root of the problem. Maybe just some condition to avoid stepping into the json scalar field?

I could work on PR if you give me some hints how you think this should be addressed.

screenshot 2017-08-21 10 57 52

@alexjgraham
Copy link

I've noticed this since updating to 1.2.0

@dbslone
Copy link

dbslone commented Aug 21, 2017

I've been running into the same issue with JSON fields when upgrading our app to Relay Modern. First occurred in 1.1.0 and upgraded to 1.2.0 and the issue still exists.

After lots of debugging I found that the reason for the failure appears to be the deepFreeze in development mode. If you set a breakpoint on line 48 and in the console type: Object.isFrozen(nextObject['data']) it will return true

@dbslone
Copy link

dbslone commented Aug 22, 2017

@jardakotesovec PR 2051 solves the issue for me. Not sure if it will be approved but for now it sovles the issue.

mike-marcacci added a commit to mike-marcacci/relay that referenced this issue Nov 15, 2017
mike-marcacci added a commit to mike-marcacci/relay that referenced this issue Nov 15, 2017
mike-marcacci added a commit to mike-marcacci/relay that referenced this issue Jan 30, 2018
mike-marcacci added a commit to mike-marcacci/relay that referenced this issue Jan 30, 2018
mike-marcacci added a commit to mike-marcacci/relay that referenced this issue Mar 16, 2018
@mike-marcacci
Copy link
Contributor

mike-marcacci commented Mar 16, 2018

For anybody else running into this, I've temporarily published to @boltline/relay-runtime. Until this or another fix get merged I'll keep this in sync with Facebook's relay-runtime releases. Once a fix is permanently in place, I'll deprecate all releases of my fork on NPM.

mike-marcacci added a commit to mike-marcacci/relay that referenced this issue Mar 16, 2018
mike-marcacci added a commit to mike-marcacci/relay that referenced this issue Mar 16, 2018
mike-marcacci added a commit to mike-marcacci/relay that referenced this issue Mar 17, 2018
@jstejada
Copy link
Contributor

jstejada commented Oct 3, 2018

@jardakotesovec, does this only happen in DEV mode?

@dbslone
Copy link

dbslone commented Oct 3, 2018

@jstejada the issue only happens in DEV as can be seen RelayMarkSweepStore:

if (__DEV__) {
deepFreeze(nextSnapshot);
}

@bejito
Copy link

bejito commented Jan 28, 2019

Hi ! I am facing the same issue. Any chance this is being fixed soon ? Or is there a way to fix this in dev mode ?

@sibelius
Copy link
Contributor

I think this is fixed on v2.0.0

@josephsavona
Copy link
Contributor

Yup, fixed in v2.0.0

@jstejada
Copy link
Contributor

Closing; if the issue persists please let us know and we’ll reopen the issue

caiosba added a commit to meedan/check-web that referenced this issue Jul 22, 2024
Implement auto-refresh for all article-related operations. Namely:

- [x] Auto-refresh fact-checks list when a new standalone fact-check is created (if it matches current filters/sort)
- [x] Auto-refresh explainers list when a new standalone explainer is created (if it matches current filters/sort)
- [x] Auto-refresh “articles” tab counter, on the item page, when an article is added/created/removed to/from an item
  - [x] Fact-check added
  - [x] Fact-check removed
  - [x] Fact-check created
  - [x] Explainer added
  - [x] Explainer removed
  - [x] Explainer created
- [x] Auto-refresh “articles” tab contents, on the item page, when an article is added/created/removed to/from an item
  - [x] First fact-check added (no articles)
  - [x] Fact-check added when item already has explainers
  - [x] Fact-check removed
  - [x] Fact-check created
  - [x] Explainer added (first)
  - [x] Explainer added (not first)
  - [x] Explainer removed
  - [x] Explainer created
- [x] Auto-refresh the fact-check card on the item page when fact-check is edited
- [x] Auto-refresh the fact-check card on the list view when fact-check is edited
- [x] Auto-refresh the explainer card on the item page when explainer is edited
- [x] Auto-refresh the explainer card on the list view when explainer is edited
- [x] Auto-disable “create fact-check” button on item page after fact-check is added
- [x] Auto-disable “create fact-check” button on item page after fact-check is created
- [x] Auto-enable “create fact-check” button on item page after fact-check is removed
- [x] Auto-refresh article card and form when report is published

Because of [this bug](facebook/relay#2049) in the Relay version we use, not all auto-refresh operations work in `development` mode. So, if you really want to test all operations above, you need to move to `production` mode, here: https://github.com/meedan/check-web/blob/feature/CV2-4915-articles-auto-refresh/gulpfile.js#L118. Even this way, not all Relay update operations are possible using Relay Compat, so for such cases I had to use callbacks.

References: CV2-4441 (epic) and CV2-4915 (ticket).
caiosba added a commit to meedan/check-web that referenced this issue Jul 22, 2024
* Explainers: rail, drawer, and navigation (#1948)

Add new rail item for Articles

Add tooltip to new rail item

Add new navigation item “Explainers” for articles

Reference: CV2-4501

* Explainers list (#1953)

List explainers under a new route /:team/articles/explainers. Requires Check API branch with the same name.

- New component <Articles /> (and unit tests) which basically reuses other existing components - the idea is to use it for both explainers (this PR) and fact-checks (coming soon in another PR).
- New component <Explainers /> that uses <Articles />.
- Extracted the paginator from the feed clusters page as a standalone component <Paginator /> now used for articles and feeds.

Reference: CV2-4500.

* Feature/cv2-4440: articles rail drawer and navigation add claims fact checks published and imported (#1954)

Refactoring: moving "Published" and "Imported" lists from Tipline rail to Articles rail
Add a new Article button component
Add a new ArticleCoreListCounter component
update list routes
Reference: CV2-4440

* Fact-checks list (#1958)

List of fact-checks.

Reference: CV2-4145.

* Epic 4441 misc updates (#1957)

* change 404 language to be less cutsey

* add new beige button themes and apply to side navigation new buttons

* update beige button colors

* add missing classname in main button for new beige color

* fix some consistency issues on the side navigation and restyle custom lists to have an easier to read layout

* add missing beige prop type to main button

* Refactoring/CV2-4450: remove claim and rating from add item dialog and make enhancements (#1959)

* Remove Claim and Rating from Add Item dialog

Remove the Claim text field from the Add item dialog

Remove the rating selection from the Add item dialog

Re-title Dialog and button from Add item to Add Media

Rename  button from 'Add Item' to 'Add Media'

Reference: CV2-4450

* Added creation forms for Explainers and Fact Checks (#1960)

* Added creation forms for Explainers and Fact Checks

ExplainerForm handles the mutations for creating or updating an explainer article. The explainer article requires title, summary, and language.  Tags and url are optional.
ClaimAndFacCheckForm handles the  mutations for creating or upating a fact check.  A claim description is first required before removing the overlay for the Fact Check section.
Form components were mostly taken from the MediaFactCheck component. An update was made to the LnaguagePicker to allow for the field to be required for the explainer.

cv2-4498 and cv2-4447

* Connect forms with links on the Articles page, made necessary updates to styling and passed down team info needed for saving any new Form

* Request pr changes for input forms

Opening another slideout from the New Article menu will close the current slideout.
Menu will close when a selection is made.
Flash message now uses the useContext hook
Changed error handling failed saves, will use the API error and fallback on a generic message
Specified the possible strings for articleType and mode values
Removed the reliance on values for TranslatedMessages which was preventing the messages from truly being translated
Removed error value that was left over from testing

* removed console statement

* Added team tags for the forms, made additional fixes to simplify value assignments.  Added tag_texts to Projects query since the New Article button lives within that object

* saving the wrong info

* List workspace tags on article card tag picker. (#1974)

Reference: CV2-4734.

* CV2-4665: filter by report information

* CV2-4665: declare propTypes for statuses

* Add articles to item from item page (#1983)

See Figma designs in referenced Jira ticket for details. But the main ideas here are:

- Delete the left sidebar from the item page (the one with claim and fact-check)
- Add a new right sidebar called "Articles"
- In that sidebar, for items with no articles (so, no explainers and no fact-check), show a button to create new article and list articles to be applied to item
- Clicking on an article (explainer or fact-check) adds it to the current item
- The articles in that right sidebar have a hover style
- When the item has an article, the right sidebar shows only the "New article" button - this case is going to be handled by another ticket

Reference: CV2-4626

* Run [full ci]

* fix merge typo/linter issue

* List styles more consistent (#1989)

* clean up some inconsistencies between lists and use more shared styles while reviewing epic progress [CV2-4441]

* remove unnecessary toolbar component [CV2-4441]

* remove unnecessary articles specific list styles [CV2-4441]

* clean up some differences between shared feed headers [CV2-4441]

* remove unnecessary import of classnames

* user shared css divider styles instead of material [CV2-4441]

* quiet down browser console error

* remove unnecessary fieldset title, now that claim has been removed

* run tests for this branch

* Fixing bad merge

* [full ci]

* Feature/4628 search for articles to add (#1990)

* Implement search existing article button and slideout

* add unit test

* add BEM class

* Add empty state for article search

---------

Co-authored-by: Brian Fleming <brianfleming@gmail.com>

* Fixes 3 bugs for the articles lists pages (#1997)

* Ticket CV2-4886: Adding titles to articles list pages

* Fixing unit test

* Ticket CV2-4889: Display number of articles on Articles tab

* Ticket CV2-4888: Move sorting on Articles lists to be inline with filters

* Fixing unit test

* Cv2 4498 edit article (#1985)

* Changes for editing a fact check and explainer

Added a new RatingSelector item
Added created_by date
Added items needed for editing to cards
Removed reliance on project_media

* Add edit to form, added additional info in edit form

edit status and publish values for fact checks
set description as summary for fact checks
claim is now claim_description
verification_statuses now sent to create form

* Added error checking for required fields
Removed trash for now, hid the footer for editing
Publish status only shows when there is an associated project media and the link is updated to go from the Articles list
Removal of Created By, name is added for Last Saved:
Pulled everything from ArticlesCard except onClick event to Articles file
The save after editing a field now uses existing FactCheck/Claim/Explainer object + new field value, rather than just the state value, to fix a most recently updated field not saving sometimes
Removed unused fields added to ArticleCard

* remove testing console log

* click on general card area, not tag buttons, added safe nav

* click on article without having to close form first

* Paired code review

---------

Co-authored-by: Brian Fleming <brianfleming@gmail.com>
Co-authored-by: Alexandre Amorim <amoedoamorim@gmail.com>

* Fixing the build (probably a bad merge)

* Card  Improvements [CV2-4887] (#1996)

* start to clean up sandbox for cluster card. Allow a fact-check link without a description in cluster cards

* add optional link and tags to article card in sandbox

* add published to sandbox article card and improve unread visuals

* hide language in shared feed on article card

* Cards fixes and tweaks

* Apply Brian's requested changes in his branch/PR and include CV2-4885 scope

* testing against api branch of same name

* sha512 instead of sha1 in package lock file

* Fix workspace ClusterCard rating in Sandbox; Fix ClusterCard unit test; Appease a couple of warnings

* improve sandbox cluster card to show zero requests in tipline lists and always show requests in shared feed

* include the Articles concept in cluster card sandbox

---------

Co-authored-by: Alexandre Amorim <amoedoamorim@gmail.com>

* Ticket CV2-4629: Managing articles from item page (#1999)

Managing articles from item page.

---------

Co-authored-by: Alexandre Amorim <amoedoamorim@gmail.com>
Co-authored-by: Brian Fleming <brianfleming@gmail.com>
Co-authored-by: Caio <117518+caiosba@users.noreply.github.com>

* Fixing things I found on the way

* Fragments and fixes

* More fixes

* Show error on blur for article forms

* Create fact-check from item page and attach to item in one go

* Create explainer from item page and add it to the item

* Replace fact-check for item

* Articles review cleanup (#2001)

* share some more styles for cards with new media article card

* tweak slideout width

* update copy on articles list alert

* Add word article to slideout titles

* better cursor behavior for cards

* Fix

* Reverting package-lock.json

* Language should be required for both Explainer and Fact-check

* Feature/cv2 4630 remove article from item (#2000)

* created remove article button

* button with confirmation modal

* Added Remove Article button, currently just for Explainer, will add Fact Check next

* added remove fact check by updating claim description project media to null

* fixed styling issues after fixing merge conflict

* wrong type for id

* Test/CV2-4883: integration tests for articles (#2003)

 Delete/Update the current failing tests for claim and fact-check
 Create fact-check from the list view and make sure it appears in the list after reload
 Create explainer from the list view and make sure it appears in the list after reload
 Add fact-check to an item on the item page and go to report page
 Add explainer to an item on the item page
 Add new classes/id's for the tests
Reference:CV2-4883

* [full ci]

* Moar articles review cleanup (#2005)

* Use consistent empty state for tags in setttings area and make table look like the rules table more

* Clean up privacy requests while reviewing Articles [CV2-4874]

* tweak empty annotations message

* update scrolling on articles list

* add tooltip for remove article button

* Change bulk actions language and limit count to the button so the user isn't confused with the pager

* add cancel button to existing article slideout

* add scroll indicator to articles list on media cluster page

* no need to close drawer on media item pages now that we have 2 columns

* All to all media in nav and header

* minor updates to new fact-check slideout to better match designs

* Refactoring/cv2 4882 use articlecard (#1998)

* Use ArticleCard in article lists powered by Search component

* Play it again, Travis

* WIP

* Imported and Published articles lists

* small copy consistency updates for claim & fact-check wording

---------

Co-authored-by: Brian Fleming <brianfleming@gmail.com>

* Articles: Auto-refresh (#2004)

Implement auto-refresh for all article-related operations. Namely:

- [x] Auto-refresh fact-checks list when a new standalone fact-check is created (if it matches current filters/sort)
- [x] Auto-refresh explainers list when a new standalone explainer is created (if it matches current filters/sort)
- [x] Auto-refresh “articles” tab counter, on the item page, when an article is added/created/removed to/from an item
  - [x] Fact-check added
  - [x] Fact-check removed
  - [x] Fact-check created
  - [x] Explainer added
  - [x] Explainer removed
  - [x] Explainer created
- [x] Auto-refresh “articles” tab contents, on the item page, when an article is added/created/removed to/from an item
  - [x] First fact-check added (no articles)
  - [x] Fact-check added when item already has explainers
  - [x] Fact-check removed
  - [x] Fact-check created
  - [x] Explainer added (first)
  - [x] Explainer added (not first)
  - [x] Explainer removed
  - [x] Explainer created
- [x] Auto-refresh the fact-check card on the item page when fact-check is edited
- [x] Auto-refresh the fact-check card on the list view when fact-check is edited
- [x] Auto-refresh the explainer card on the item page when explainer is edited
- [x] Auto-refresh the explainer card on the list view when explainer is edited
- [x] Auto-disable “create fact-check” button on item page after fact-check is added
- [x] Auto-disable “create fact-check” button on item page after fact-check is created
- [x] Auto-enable “create fact-check” button on item page after fact-check is removed
- [x] Auto-refresh article card and form when report is published

Because of [this bug](facebook/relay#2049) in the Relay version we use, not all auto-refresh operations work in `development` mode. So, if you really want to test all operations above, you need to move to `production` mode, here: https://github.com/meedan/check-web/blob/feature/CV2-4915-articles-auto-refresh/gulpfile.js#L118. Even this way, not all Relay update operations are possible using Relay Compat, so for such cases I had to use callbacks.

References: CV2-4441 (epic) and CV2-4915 (ticket).

* [full ci]

* Refactoring/cv2 4944 cleanup code (#2010)

* Cleanup WIP

* Display status report button disabled in slideout

* Show report status button in slideout; Click report status icon in card to edit

* Remove unwanted comments

---------

Co-authored-by: Daniele Valverde <34126648+danielevalverde@users.noreply.github.com>
Co-authored-by: Caio Almeida <117518+caiosba@users.noreply.github.com>
Co-authored-by: Brian Fleming <brianfleming@gmail.com>
Co-authored-by: Brian Fleming <brianfleming@users.noreply.github.com>
Co-authored-by: Sarah Kehoe <161879347+sarahkeh@users.noreply.github.com>
Co-authored-by: Sawy <melsawy.be7eri@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants